Use Yarn Workspaces #11252

gaearon · 2017-10-17T14:13:27Z

Here’s an overview of what happened:

src/ is gone.
packages/* is now the source of truth.
Running yarn in the root sets up the symlinks in root node_modules.
Sources live in packages/*/src. I intentionally haven’t touched them.
Entry points in packages/*/*.js redirect to the sources. They are used for Jest and Rollup.
Entry points in packages/*/npm/*.js redirect to flat bundles. They are used for npm.
For now, you can still use Haste, but I plan to tighten it up (see future work below).
Internal fixtures now only work with Yarn, not npm. We depend on it anyway.
The +13K line diff is due to a bunch of lockfiles I added to fixtures.

Must Do

Move all code
Fix the build
Update all configs referencing src/
Why did some files get lost first time I committed? Any bad gitignores?
Fix release script
Fix fixtures now that NODE_PATH doesn't work

hzoo · 2017-10-18T00:25:30Z

packages/react-art/npm/lib/Circle.art.js

@@ -1,6 +1,18 @@
 'use strict';

-var _extends = Object.assign || function (target) { for (var i = 1; i < arguments.length; i++) { var source = arguments[i]; for (var key in source) { if (Object.prototype.hasOwnProperty.call(source, key)) { target[key] = source[key]; } } } return target; };
+var _extends =
+  Object.assign ||


Hmm noticed this, does this assume a object.assign polyfill (can use useBuiltIns option if so)

It doesn't assume it. (This is very rarely used code though and doesn't matter for majority of users. So we just checked in a compiled version.)

trueadm

Everything looks good to me. Two things though:

There was lots of file moving/renaming, we should be careful we don't lose any git history as I've had issues with this in the past (maybe a non-issue, but wanted to flag it anyway).
Can we block NPM usage so contributors don't run into issues (as this is all tied to Yarn now)?

bvaughn

This looks great Dan!

bvaughn · 2017-10-18T16:34:16Z

packages/react-native/index.js

+
+'use strict';
+
+// TODO: This package is a lie. Maybe rename to react-native-renderer?


bvaughn · 2017-10-18T16:39:51Z

scripts/prettier/index.js

+      // Internal forwarding modules
+      'packages/*/*.js',
+      // Source files
+      'packages/*/src/**/*.js',


~~Should we also include packages/*/npm/*.js to ensure the NPM entry points stay formatted?~~

Nevermind. Looks like these are covered below in scripts

bvaughn · 2017-10-18T16:41:39Z

scripts/release/config.js

@@ -2,6 +2,7 @@

 const dependencies = ['fbjs', 'object-assign', 'prop-types'];

+// TODO: enumerate all non-private package folders in packages/*?


👍 I like this.

We could actually enumerate all package folders (so we increment version numbers) but just only publish non-private ones.

bvaughn · 2017-10-18T16:47:29Z

scripts/rollup/packaging.js

+    return Promise.resolve();
+  }
+  if (fs.existsSync(to)) {
+    // We already created this package (e.g. due to another entry point).


~~Are we concerned about a partial copy? (Like maybe something failed from a previous run?)~~

Nevermind. Looks like the main rollup/build script calls rimraf before createBundle.

bvaughn · 2017-10-18T16:48:04Z

scripts/rollup/packaging.js

    return Promise.resolve();
  }
+  // TODO: verify that all copied files are either in the "files"
+  // whitelist or implicitly published by npm.


This is something the release script could do too, if it makes more sense there.

This makes the test pass again, but breaks the build because npm/ folders aren't used yet. I'm not sure if we'll keep this structure--I'll just keep working and fix the build after it settles down.

Note that this is currently broken because Jest ignores node_modules, and so Yarn linking makes Jest skip React source when transforming.

It is now unnecessary. Some tests fail though.

Jest sees node_modules and thinks it's third party code. This is a hacky way to teach Jest to still transform anything in node_modules/react* if it resolves outside of node_modules (such as to our packages/*) folder. I'm not very happy with this and we should revisit.

It's not super clear how to organize this properly yet.

I didn't change this file but Flow started complaining. Caleb said this annotation was unnecessarily using $Abstract though so I removed it.

Use file: instead of NODE_PATH since NODE_PATH. NODE_PATH trick only worked because we had no react/react-dom in root node_modules, but now we do. file: dependency only works as I expect in Yarn, so I moved the packaging fixtures to use Yarn and committed lockfiles. Verified that the page shows up.

blling · 2017-10-19T00:06:10Z

Wow, really a lot of changs, great work!
but why to refrustructure it?

gaearon · 2017-10-19T00:07:20Z

So that package boundaries are clearer.

anhuiliujun · 2017-10-19T14:37:07Z

great work!

flaviolivolsi · 2017-10-21T17:57:17Z

I updated reactjs.org docs following this major change in the folder structure, feedbacks and improvements welcome :)

* Enable Yarn workspaces for packages/* * Move src/isomorphic/* into packages/react/src/* * Create index.js stubs for all packages in packages/* This makes the test pass again, but breaks the build because npm/ folders aren't used yet. I'm not sure if we'll keep this structure--I'll just keep working and fix the build after it settles down. * Put FB entry point for react-dom into packages/* * Move src/renderers/testing/* into packages/react-test-renderer/src/* Note that this is currently broken because Jest ignores node_modules, and so Yarn linking makes Jest skip React source when transforming. * Remove src/node_modules It is now unnecessary. Some tests fail though. * Add a hacky workaround for Jest/Workspaces issue Jest sees node_modules and thinks it's third party code. This is a hacky way to teach Jest to still transform anything in node_modules/react* if it resolves outside of node_modules (such as to our packages/*) folder. I'm not very happy with this and we should revisit. * Add a fake react-native package * Move src/renderers/art/* into packages/react-art/src/* * Move src/renderers/noop/* into packages/react-noop-renderer/src/* * Move src/renderers/dom/* into packages/react-dom/src/* * Move src/renderers/shared/fiber/* into packages/react-reconciler/src/* * Move DOM/reconciler tests I previously forgot to move * Move src/renderers/native-*/* into packages/react-native-*/src/* * Move shared code into packages/shared It's not super clear how to organize this properly yet. * Add back files that somehow got lost * Fix the build * Prettier * Add missing license headers * Fix an issue that caused mocks to get included into build * Update other references to src/ * Re-run Prettier * Fix lint * Fix weird Flow violation I didn't change this file but Flow started complaining. Caleb said this annotation was unnecessarily using $Abstract though so I removed it. * Update sizes * Fix stats script * Fix packaging fixtures Use file: instead of NODE_PATH since NODE_PATH. NODE_PATH trick only worked because we had no react/react-dom in root node_modules, but now we do. file: dependency only works as I expect in Yarn, so I moved the packaging fixtures to use Yarn and committed lockfiles. Verified that the page shows up. * Fix art fixture * Fix reconciler fixture * Fix SSR fixture * Rename native packages

facebook-github-bot added the CLA Signed label Oct 17, 2017

gaearon force-pushed the new-struc branch 7 times, most recently from 712df01 to eae492d Compare October 17, 2017 22:47

bvaughn self-requested a review October 17, 2017 23:02

gaearon changed the title ~~[WIP] Move to Yarn Workspaces~~ Use Yarn Workspaces Oct 17, 2017

hzoo reviewed Oct 18, 2017

View reviewed changes

gaearon requested review from bvaughn and trueadm and removed request for bvaughn October 18, 2017 09:56

trueadm approved these changes Oct 18, 2017

View reviewed changes

gaearon mentioned this pull request Oct 18, 2017

Add static injection for feature flags #11269

Merged

bvaughn approved these changes Oct 18, 2017

View reviewed changes

bvaughn self-requested a review October 18, 2017 16:56

bvaughn approved these changes Oct 18, 2017

View reviewed changes

gaearon force-pushed the new-struc branch 2 times, most recently from 5060247 to 228e270 Compare October 18, 2017 22:14

gaearon added 9 commits October 19, 2017 00:11

Enable Yarn workspaces for packages/*

cafd508

Move src/isomorphic/* into packages/react/src/*

fd6784c

Create index.js stubs for all packages in packages/*

24260e8

This makes the test pass again, but breaks the build because npm/ folders aren't used yet. I'm not sure if we'll keep this structure--I'll just keep working and fix the build after it settles down.

Put FB entry point for react-dom into packages/*

60a7518

Move src/renderers/testing/* into packages/react-test-renderer/src/*

b3ccdbb

Note that this is currently broken because Jest ignores node_modules, and so Yarn linking makes Jest skip React source when transforming.

Remove src/node_modules

1c62fd8

It is now unnecessary. Some tests fail though.

Add a fake react-native package

c5e1470

Move src/renderers/art/* into packages/react-art/src/*

165d574

gaearon added 21 commits October 19, 2017 00:11

Move src/renderers/dom/* into packages/react-dom/src/*

8dcb375

Move src/renderers/shared/fiber/* into packages/react-reconciler/src/*

82510db

Move DOM/reconciler tests I previously forgot to move

b50aa80

Move src/renderers/native-*/* into packages/react-native-*/src/*

30e7d8c

Move shared code into packages/shared

a9f79b3

It's not super clear how to organize this properly yet.

Add back files that somehow got lost

5c31f26

Fix the build

6634d37

Prettier

f1aefeb

Add missing license headers

f179c2b

Fix an issue that caused mocks to get included into build

2917a68

Update other references to src/

c07109a

Re-run Prettier

6838cc9

Fix lint

af93d31

Fix weird Flow violation

5cc7de4

I didn't change this file but Flow started complaining. Caleb said this annotation was unnecessarily using $Abstract though so I removed it.

Update sizes

fa32739

Fix stats script

d714d38

Fix art fixture

dbe6737

Fix reconciler fixture

48ebbf7

Fix SSR fixture

f9d855c

Rename native packages

b08673f

gaearon force-pushed the new-struc branch from c6205ca to b08673f Compare October 18, 2017 23:11

gaearon merged commit d9c1dbd into facebook:master Oct 18, 2017

gaearon deleted the new-struc branch October 18, 2017 23:22

gaearon mentioned this pull request Oct 18, 2017

Update docs referencing src/ reactjs/react.dev#181

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Yarn Workspaces #11252

Use Yarn Workspaces #11252

gaearon commented Oct 17, 2017 •

edited

Loading

hzoo Oct 18, 2017

gaearon Oct 18, 2017

trueadm left a comment

bvaughn left a comment

bvaughn Oct 18, 2017

bvaughn Oct 18, 2017

bvaughn Oct 18, 2017

bvaughn Oct 18, 2017

bvaughn Oct 18, 2017

blling commented Oct 19, 2017

gaearon commented Oct 19, 2017

anhuiliujun commented Oct 19, 2017

flaviolivolsi commented Oct 21, 2017 •

edited

Loading


		'use strict';

		// TODO: This package is a lie. Maybe rename to react-native-renderer?

		@@ -2,6 +2,7 @@

		const dependencies = ['fbjs', 'object-assign', 'prop-types'];

		// TODO: enumerate all non-private package folders in packages/*?

Use Yarn Workspaces #11252

Use Yarn Workspaces #11252

Conversation

gaearon commented Oct 17, 2017 • edited Loading

Must Do

hzoo Oct 18, 2017

Choose a reason for hiding this comment

gaearon Oct 18, 2017

Choose a reason for hiding this comment

trueadm left a comment

Choose a reason for hiding this comment

bvaughn left a comment

Choose a reason for hiding this comment

bvaughn Oct 18, 2017

Choose a reason for hiding this comment

bvaughn Oct 18, 2017

Choose a reason for hiding this comment

bvaughn Oct 18, 2017

Choose a reason for hiding this comment

bvaughn Oct 18, 2017

Choose a reason for hiding this comment

bvaughn Oct 18, 2017

Choose a reason for hiding this comment

blling commented Oct 19, 2017

gaearon commented Oct 19, 2017

anhuiliujun commented Oct 19, 2017

flaviolivolsi commented Oct 21, 2017 • edited Loading

gaearon commented Oct 17, 2017 •

edited

Loading

flaviolivolsi commented Oct 21, 2017 •

edited

Loading