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

Build distinct dev / prod & min builds #7242

Closed
wants to merge 1 commit into from

Conversation

zpao
Copy link
Member

@zpao zpao commented Jul 11, 2016

Hopefully this helps reduce some confusion about what each build actually is and maybe we'll see fewer people doing benchmarks with the dev version.

There's some more to do to make sure this is actually right and works, but this is the general idea. We'd build react.dev.js and react.prod.js and the minified version of each. We should do all the other ones as well (addons less important long term, but react-dom definitely needs this treatment because #7164).

We start recommending these file names as soon as they are part of a release. We keep building the plain react.js & react.min.js for compatibility. For v16 we stop building them entirely.

TODO:

  • Remove the minified dev-mode warning. It won't make any sense. We might still want it for npm though
  • Make sure prod build isn't already minified. It mostly is because of the uglifify step. We might not need those anymore anyway if we do this after rolloup (that step is only there to remove some requires but rollup handles that I think).
  • docs updates
  • Do 2 builds instead of 5 (should be able to just run uglify over the non-min code, can even have a .map then, and then copy the right files instead of doing a duplicate build).

@quantizor
Copy link
Contributor

quantizor commented Jul 11, 2016

Not sure I agree with changing the names of both. Why not let react.js/react.min.js imply production (like all other JS libraries) and offer a react.dev.js/react.dev.min.js as an alternative?

@zpao
Copy link
Member Author

zpao commented Jul 11, 2016

If we had done this from the beginning, I'd agree. But the builds have existed like this for a long time and I'm afraid there would be more confusion from reusing the current files names (especially with things like npmcdn, bower). Plus, I kind of like that the use of .prod implies there is a .dev, whereas the alternative would not pique anybody's curiosity and they may think prod builds are the only thing.

We could maybe do that but it would have to happen at v16 and not before. I don't know when that will be. Maybe we could wait but it would mean living with the current confusion for longer.

@quantizor
Copy link
Contributor

@zpao is the confusion so pervasive that it necessitates a major ergonomic change? Personally have never heard of this being an issue, but you folks definitely sample the feedback at a bigger scale.

@gaearon
Copy link
Collaborator

gaearon commented Jul 11, 2016

I see a new benchmark using DEV version every other week. The confusion unfortunately is pervasive.

@sebmarkbage
Copy link
Collaborator

Would it be enough to just rename react.min.js to react.optimized.js instead of splitting them out?

What if we use a build tool that inlines and minifies as a single batch then we can't keep separating them? There should be no reason to use one but not the other.

@gaearon
Copy link
Collaborator

gaearon commented Jul 11, 2016

Would it be enough to just rename react.min.js to react.optimized.js instead of splitting them out?

I think the confusion is the other way around. People see a URL like https://cdnjs.cloudflare.com/ajax/libs/react/15.2.1/react.js and it doesn’t seem immediately wrong. So it’s the one that should be react.unoptimized.js.

@keyz
Copy link
Contributor

keyz commented Jul 11, 2016

There are a few things that Rollup itself can't prune. For example ReactComponentTreeDevTool still shows up in the unminified prod build and it will be removed by the uglify step (the Rollup official documentation also recommends using uglify together with Rollup). Not 100% sure why but it might because of self-references.

@trueadm
Copy link
Contributor

trueadm commented Apr 5, 2017

Closing this PR due to the work that's happened around Rollup and flat bundles on #9327

@trueadm trueadm closed this Apr 5, 2017
@gaearon
Copy link
Collaborator

gaearon commented Apr 5, 2017

For now we settled with @trueadm on using react.development.js and react.production.min.js but happy to change if the team doesn’t agree.

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.

None yet

6 participants