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

Move ReactFiberTreeReflection to react-reconciler/reflection #11683

Merged
merged 7 commits into from Nov 28, 2017

Conversation

@adamsau
Copy link
Contributor

@adamsau adamsau commented Nov 28, 2017

ref #11659

@@ -10,7 +10,8 @@
"fbjs": "^0.8.16",
"object-assign": "^4.1.1",
"prop-types": "^15.6.0",
"regenerator-runtime": "^0.11.0"
"regenerator-runtime": "^0.11.0",
"react-reconciler": "^0.6.0"

This comment has been minimized.

@gaearon

gaearon Nov 28, 2017
Member

Our release script probably won't know to bump it, so eventually the local version will stop satisfying it. Can we use "*" as a version instead? Since we never publish it, that should be enough as a workaround.

// Private. Used only by fixtures/fiber-debugger.
ReactFiberInstrumentation,
},
__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED: {},

This comment has been minimized.

@gaearon

gaearon Nov 28, 2017
Member

We can just remove this field for now then.


'use strict';

export * from './src/ReactFiberTreeReflection';

This comment has been minimized.

@gaearon

gaearon Nov 28, 2017
Member

Can we use a require here for consistency with other top level entry points?
i.e.

module.exports = require('./src/ReactFiberTreeReflection');

We can eventually change this but I'd like all entry points to look the same.

This comment has been minimized.

@gaearon

gaearon Nov 28, 2017
Member

Nvm, I see why this wouldn't work now.

@adamsau
Copy link
Contributor Author

@adamsau adamsau commented Nov 28, 2017

thanks for the review! Yes i notice the require consistency but i find export to be a more easy and neat way to handle~

@gaearon
Copy link
Member

@gaearon gaearon commented Nov 28, 2017

Could you please rebase on master and then re-run yarn build to update JSON file with build stats?

adamsau and others added 6 commits Nov 28, 2017
We don't know the latest local version, and release script currently doesn't bump deps automatically.
I didn't realize it would break the build.
@gaearon
Copy link
Member

@gaearon gaearon commented Nov 28, 2017

Never mind, I'm doing it now.

@gaearon gaearon force-pushed the adamsau:master branch from 036fb85 to f4b209f Nov 28, 2017
They're unnecessary now that we run real tests on reconciler bundles.
@gaearon gaearon merged commit 8e876d2 into facebook:master Nov 28, 2017
1 check passed
1 check passed
ci/circleci Your tests passed on CircleCI!
Details
@gaearon
Copy link
Member

@gaearon gaearon commented Nov 28, 2017

This was great. Thank you so much!

@adamsau
Copy link
Contributor Author

@adamsau adamsau commented Nov 28, 2017

welcome!

NMinhNguyen referenced this pull request in NMinhNguyen/react-shallow-renderer Jan 29, 2020
* Move ReactFiberTreeReflection to react-reconciler/reflection #11659

* Use * for react-reconciler

We don't know the latest local version, and release script currently doesn't bump deps automatically.

* Remove unused field

* Use CommonJS in entry point for consistency

* Undo the CommonJS change

I didn't realize it would break the build.

* Record sizes

* Remove reconciler fixtures

They're unnecessary now that we run real tests on reconciler bundles.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.