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

Establish a convention for forcing jsx rebuilds #535

Merged
merged 2 commits into from
Dec 4, 2013

Conversation

benjamn
Copy link
Contributor

@benjamn benjamn commented Nov 13, 2013

Pull request #526 updated the behavior of vendor/constants.js without changing any source files or the bin/jsx-internal script, so files that should have been rebuilt (like utils/__tests__/ImmutableObject-test.js) were not automatically rebuilt (unless you knew to do rm -rf .module-cache manually).

This commit allows us to bump a version number (commonerCacheBuster) when we know the transform toolchain has been altered in a way that will not be visible to commoner/jsx.

With this convention, if we reset to an older revision (e.g. during a git bisect) and the appropriate cached module files are still in the .module-cache/, they can be used without rebuilding. That's why I prefer this approach to just deleting the .module-cache/.

cc @subtleGradient @zpao

Closes #104.
Closes #496.
Closes #530.

@@ -2,6 +2,10 @@

var grunt = require('grunt');

// Increment this number to trigger a full rebuild in rare cases when the
// JSX transform toolchain is altered in ways not visible to Commoner/JSX.
var commonerCacheBuster = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

So, in order for this to work we'll need to remember to modify this value when one of those files changes?
How will we remember to do that?
Seems like the sort of thing that will happen a few times and then we'll forget about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to turn that question back to you: can you think of a good way to remind us, or a better solution? This pull request makes it possible for one mindful person to prevent confusion for everyone, and that's a step in the right direction.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point.

Copy link
Member

Choose a reason for hiding this comment

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

Can you make this a "CONSTANT"?

Also, is this something that we want to put into package.json and read from there in bin/jsx[-internal]? The change that prompted this wouldn't matter publicly but we talked about that before (changes to jsx formatting, which are likely to come soon).

@zpao
Copy link
Member

zpao commented Dec 4, 2013

ping

Pull request facebook#526 updated the behavior of vendor/constants.js without
changing any source files or the bin/jsx-internal script, so files that
should have been rebuilt (like utils/__tests__/ImmutableObject-test.js)
were not automatically rebuilt (unless you knew to do `grunt clean` or
`rm -rf .module-cache` manually).

This commit allows us to bump a version number when we know the transform
toolchain has been altered in a way that will not be visible to
commoner/jsx.

With this convention, if we reset to an older revision (e.g. during a git
bisect) and the appropriate cached module files are still in the
.module-cache/, they can be used without rebuilding. That's why I prefer
this approach to just deleting the .module-cache/.

Closes facebook#104.
Closes facebook#496.
Closes facebook#530.
@subtleGradient
Copy link
Contributor

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants