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

Remove cache-invalidating code/hack #10615

Closed
nixolas1 opened this issue Sep 6, 2017 · 5 comments
Closed

Remove cache-invalidating code/hack #10615

nixolas1 opened this issue Sep 6, 2017 · 5 comments

Comments

@nixolas1
Copy link

nixolas1 commented Sep 6, 2017

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
In the output code after building there is a hack in the code which includes the time of compilation, which causes the whole file to be modified, which when code-splitting is bad for caching.

Relevant output code (see LAST_BUILD):

if (typeof process !== 'undefined' && Object({"NODE_ENV":"development","LAST_BUILD":"2017-09-06T07:53:58.689Z"}) && "development" === 'test') {
  // Temporary hack.
  // Inline requires don't work well with Jest:
  // https://github.com/facebook/react/issues/7240
  // Remove the inline requires when we don't need them anymore:
  // https://github.com/facebook/react/pull/7178
  ReactComponentTreeHook = __webpack_require__("./node_modules/react/lib/ReactComponentTreeHook.js");
}

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar (template: https://jsfiddle.net/ebsrpraL/).
Build project with react and react-dom in development or production, check diff between two supposedly identical builds.

What is the expected behavior?
The two builds outputs should be identical if no modifications have been made.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
15.6.1

@nhunzaker
Copy link
Contributor

I'm not too familiar with the backstory, but I'm curious if we can eliminate this now that #7178 has merged.

@nixolas1
Copy link
Author

nixolas1 commented Sep 6, 2017

I found that LAST_BUILD was actually a variable we passed into process.env when building it through webpack. Removing it solved my cacheing issue at least, but I think removing the hack would mitigate such future bugs.

@nhunzaker
Copy link
Contributor

Ah... interesting. This is because of:

if(
  typeof process !== 'undefined' &&
  process.env &&
  process.env.NODE_ENV === 'test'
)

In these places:

https://github.com/facebook/react/blob/master/src/renderers/shared/stack/reconciler/flattenStackChildren.js#L24-L28

https://github.com/facebook/react/blob/master/src/renderers/shared/stack/reconciler/ReactChildReconciler.js#L27-L31

Looks like your process.env is getting inlined. I'm guessing because of the use of webpack DefinePlugin or EnvironmentPlugin.

This shouldn't be a problem in React 16, since the stack renderer is going away.

In any case, I think htis code path should be stripped away by Uglify. Could you confirm for me if this is showing up in your production build?

@nixolas1
Copy link
Author

nixolas1 commented Sep 6, 2017

Yes, we were using a DefinePlugin with process.env things inside.
I can confirm the code was still present in the production build, after uglifying.

@gaearon
Copy link
Collaborator

gaearon commented Sep 6, 2017

Thanks for reporting!

This is fixed in master (basically that file won't be shipped at all in React 16). You can already try React 16 beta if you'd like.

I don't think we will be making further changes to this in 15.

@gaearon gaearon closed this as completed Sep 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants