Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reorganize code structure #11288

Merged
merged 7 commits into from
Oct 19, 2017
Merged

Reorganize code structure #11288

merged 7 commits into from
Oct 19, 2017

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Oct 19, 2017

A few changes as a followup to Workspaces.

  • Entry points now have natural names, e.g. React, ReactDOM. Just like before Fiber.
  • We now use npm package entry points when importing in the few places we weren't. Such as ReactFiberReconciler -> react-reconciler.
  • Since DOM tests often test both client and server, I removed the mostly arbitary distinction and put most of them together into packages/dom/__tests__.
  • For DOM implementation, I did another pass at checking which files are client-only/server-only/shared, and separated them into dom/client, dom/server, and dom/shared.
  • I deleted a few dead files.
  • I moved FB-specific @preventMunge directives into flat bundle itself instead of individual source files.
  • I separated TestUtils and ReactDOMUnstableNativeDependencies into a separate bundle type called RENDERER_UTILS. The main difference is it shouldn't bundle a reconciler inside itself (unlike a real renderer). This is how I uncovered React Test Utils shouldn't bundle a copy of the reconciler #11280 (which I fixed in Only renderers should depend on reconciler code #11281).

The build products look mostly the same to me except for minor dependency order changes.
Bundle fixture works.

screen shot 2017-10-19 at 6 31 10 pm

Also verified that UFI and Power Editor work in DEV and PROD.

@bvaughn
Copy link
Contributor

bvaughn commented Oct 19, 2017

For DOM implementation, I did another pass at checking which files are client-only/server-only/shared, and separated them into dom/client, dom/server, and dom/shared.

Nice!

@@ -41,7 +40,7 @@ ReactFiberErrorLogger.injection.injectDialog(
ReactNativeFiberErrorDialog.showDialog,
);

const ReactNativeFiber: ReactNativeType = {
const ReactNativeRenderer: ReactNativeType = {
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a bit confusing that we now have ReactNativeRenderer that requires a file named ReactNativeFiberRenderer.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agree. I’m also not over the moon with arbitrary ReactDOM/ReactDOMComponent separation. Need to do a pass restructuring those later.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, it's def not a blocker to this PR. Just an observation that the naming seems even weirder now 😄

const RENDERER = moduleTypes.RENDERER;
// Helper packages that access specific renderer's internals. (e.g. TestUtils)
const RENDERER_UTILS = moduleTypes.RENDERER_UTILS;
// Standalone reconciler for third-party renderers.
const RECONCILER = moduleTypes.RECONCILER;
Copy link
Contributor

Choose a reason for hiding this comment

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

Love it ^

@@ -9,6 +9,7 @@ function getProvidesHeader(hasteFinalName) {
*
* @noflow
* @providesModule ${hasteFinalName}
* @preventMunge
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

I forgot to @flow-ify those files. This uncovered some issues.
Prettier, I love you but you're bringing me down

Like a rat in a cage
Pulling minimum wage
Prettier, I love you but you're bringing me down

Prettier, you're safer and you're wasting my time
Our records all show you were filthy but fine
But they shuttered your stores
When you opened the doors
To the cops who were bored once they'd run out of crime

Prettier, you're perfect, oh, please don't change a thing
Your mild billionaire mayor's now convinced he's a king
So the boring collect
I mean all disrespect
In the neighborhood bars I'd once dreamt I would drink

Prettier, I love you but you're freaking me out
There's a ton of the twist but we're fresh out of shout
Like a death in the hall
That you hear through your wall
Prettier, I love you but you're freaking me out

Prettier, I love you but you're bringing me down
Prettier, I love you but you're bringing me down
Like a death of the heart
Jesus, where do I start?
But you're still the one pool where I'd happily drown

And oh! Take me off your mailing list
For kids who think it still exists
Yes, for those who think it still exists
Maybe I'm wrong and maybe you're right
Maybe I'm wrong and maybe you're right
Maybe you're right, maybe I'm wrong
And just maybe you're right

And oh! Maybe mother told you true
And there'll always be somebody there for you
And you'll never be alone
But maybe she's wrong and maybe I'm right
And just maybe she's wrong
Maybe she's wrong and maybe I'm right
And if so, here's this song!
@gaearon
Copy link
Collaborator Author

gaearon commented Oct 19, 2017

Only prettier failed last time so this is good to go.

@gaearon gaearon merged commit 3136115 into facebook:master Oct 19, 2017
HostConfig,
Deadline,
Reconciler,
} from './src/ReactFiberReconciler';
Copy link
Contributor

Choose a reason for hiding this comment

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

Great song by the way 😆

NMinhNguyen referenced this pull request in enzymejs/react-shallow-renderer Jan 29, 2020
* Move files and tests to more meaningful places

* Fix the build

Now that we import reconciler via react-reconciler, I needed to make a few tweaks.

* Update sizes

* Move @preventMunge directive to FB header

* Revert unintentional change

* Fix Flow coverage

I forgot to @flow-ify those files. This uncovered some issues.

* Prettier, I love you but you're bringing me down
Prettier, I love you but you're bringing me down

Like a rat in a cage
Pulling minimum wage
Prettier, I love you but you're bringing me down

Prettier, you're safer and you're wasting my time
Our records all show you were filthy but fine
But they shuttered your stores
When you opened the doors
To the cops who were bored once they'd run out of crime

Prettier, you're perfect, oh, please don't change a thing
Your mild billionaire mayor's now convinced he's a king
So the boring collect
I mean all disrespect
In the neighborhood bars I'd once dreamt I would drink

Prettier, I love you but you're freaking me out
There's a ton of the twist but we're fresh out of shout
Like a death in the hall
That you hear through your wall
Prettier, I love you but you're freaking me out

Prettier, I love you but you're bringing me down
Prettier, I love you but you're bringing me down
Like a death of the heart
Jesus, where do I start?
But you're still the one pool where I'd happily drown

And oh! Take me off your mailing list
For kids who think it still exists
Yes, for those who think it still exists
Maybe I'm wrong and maybe you're right
Maybe I'm wrong and maybe you're right
Maybe you're right, maybe I'm wrong
And just maybe you're right

And oh! Maybe mother told you true
And there'll always be somebody there for you
And you'll never be alone
But maybe she's wrong and maybe I'm right
And just maybe she's wrong
Maybe she's wrong and maybe I'm right
And if so, here's this song!
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