Skip to content

providesModule -> explicit requires for cross-package dependencies#9078

Merged
sebmarkbage merged 4 commits into
react:masterfrom
sebmarkbage:forwardingmodules
Mar 1, 2017
Merged

providesModule -> explicit requires for cross-package dependencies#9078
sebmarkbage merged 4 commits into
react:masterfrom
sebmarkbage:forwardingmodules

Conversation

@sebmarkbage

@sebmarkbage sebmarkbage commented Feb 28, 2017

Copy link
Copy Markdown
Contributor

This is one step closer to a CommonJS/Lerna set up.

I added some forwarding modules for Flow and Jest's sake. They have the same purpose as the symlinks Lerna or Yarn Workspaces would provide.

I ran a codemod to use explicit package paths for cross-package dependencies and unit tests.

The rest of the requires are still providesModule.

@trueadm trueadm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :)

@gaearon

gaearon commented Feb 28, 2017

Copy link
Copy Markdown
Collaborator

Need to verify UMD builds still work correctly.
I'll take a look soon.

@trueadm

trueadm commented Feb 28, 2017

Copy link
Copy Markdown
Contributor

@gaearon maybe wait till you come into the office, I'd like you to step me through that process to see if there's anything I missed? Thanks man.

@gaearon

gaearon commented Feb 28, 2017

Copy link
Copy Markdown
Collaborator

Yea that was my plan!

@trueadm

trueadm commented Feb 28, 2017

Copy link
Copy Markdown
Contributor

@gaearon @sebmarkbage I've just tested the UMD bundles locally on your branch and they all look good to me!

Files that require modules from a different package than their own now
does so by the npm path name instead of the providesModule.
This is a bit lame but because of our module rewrite we need to white
list all the paths that we don't *don't* want to rewrite.

@sophiebits sophiebits left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels weird to require react/lib/* from files that get packaged into react, but I suppose this makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants