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

Add a script for extracting error codes #6882

Merged
merged 7 commits into from Jun 1, 2016
Merged

Conversation

keyz
Copy link
Contributor

@keyz keyz commented May 26, 2016

As we talked in #6874, this is the first step that extracts error codes to a JSON file. The script adds any new error messages to the end of the file without changing existing codes. The traversal order of files is determined by their path names so that the result is stable.

I added babel-traverse and babylon as new dev dependencies, but they are already required by babel-core so we actually just lifted them up.

Thanks for reviewing! CC @spicyj

A few things left:

  • Rename files
  • Require glob patterns rather than copy them Use gulp

@ghost
Copy link

ghost commented May 26, 2016

@keyanzhang updated the pull request.

var source = fs.readFileSync(filePath, 'utf8');
return babylon.parse(source, {
sourceType: 'module',
plugins: [
Copy link
Contributor

Choose a reason for hiding this comment

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

should this use the same config from 'babel-preset-fbjs' in #6876?

Copy link
Contributor Author

@keyz keyz May 27, 2016

Choose a reason for hiding this comment

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

Hey, thanks for catching this! This is kinda tricky -- as a parser, babylon has its own options here and it's different from babel's so we can't directly import/require it. All our syntax (parser)-related configs are specified here in fbjs. I'll update this one to make sure they are the same.

@facebook-github-bot
Copy link

@keyanzhang updated the pull request.

@keyz
Copy link
Contributor Author

keyz commented May 27, 2016

@zpao the tests pass locally but not on travis. Could this be a caching-related issue? https://travis-ci.org/facebook/react/jobs/133367229

@@ -95,6 +97,7 @@
"testPathDirs": [
"<rootDir>/eslint-rules",
"<rootDir>/mocks",
"<rootDir>/scripts/error-extractor",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's just add all of scripts here in case we want to add more later.

@facebook-github-bot
Copy link

@keyanzhang updated the pull request.


var evalToString = require('./evalToString');
var invertObject = require('./invertObject');
var safeParseJSON = require('./safeParseJSON');
Copy link
Collaborator

Choose a reason for hiding this comment

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

As discussed in person, let's move this back inline – but move all the logic in this file into top-level functions and then just call it at the end of the file.

@facebook-github-bot
Copy link

@keyanzhang updated the pull request.

@keyz
Copy link
Contributor Author

keyz commented Jun 1, 2016

@zpao @spicyj I rewrote the script to a gulp plugin. Would you take a look and let me know what you think? Thanks!

@facebook-github-bot
Copy link

@keyanzhang updated the pull request.

@sophiebits
Copy link
Collaborator

This looks great. Two things:

  1. Can we call the directory "error-codes" not "error-code"?
  2. Can we make Travis run the gulp script to make sure it runs without crashing? You can add it after this git checkout line:
    git checkout -- src/renderers/dom/shared/ReactDOMFeatureFlags.js
    .

Otherwise this looks great – feel free to merge whenever.

function flush(cb) {
fs.writeFile(
errorMapFilePath,
JSON.stringify(invertObject(existingErrorMap), null, 2) + os.EOL,
Copy link
Member

Choose a reason for hiding this comment

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

I think we probably actually want to just use '\n' so we generate the same file on Windows. Or even just leave off the newline.

@ghost
Copy link

ghost commented Jun 1, 2016

@keyanzhang updated the pull request.

@keyz keyz merged commit bfd1531 into facebook:master Jun 1, 2016
@keyz keyz deleted the error-extractor branch June 1, 2016 18:21
@ghost
Copy link

ghost commented Jun 1, 2016

@keyanzhang updated the pull request.

This was referenced Jun 1, 2016
keyz added a commit that referenced this pull request Jun 7, 2016
@zpao zpao added this to the 15-next milestone Jun 8, 2016
@keyz keyz mentioned this pull request Jun 8, 2016
7 tasks
zpao pushed a commit that referenced this pull request Jun 14, 2016
@zpao zpao modified the milestones: 15-next, 15.2.0 Jun 14, 2016
dgoldstein0 pushed a commit to dropbox/react that referenced this pull request Jul 6, 2016
…help from 1d0c1b1 to get the basic gulp setup (0.14 beta v2 code), and took a lot of dependency wrangling.

`grunt extract-errors` should run the script now.  I'm starting with a fresh codes.json because these will never match up with the official ones unfortunately.

Things skipped:
- travis integration.  I don't know anything about travis so don't know if / how to hook this up for it.
- inserting this as part of a full build process.  It looks like it's a oneoff in master too fwiw
christoffer-dropbox pushed a commit to christoffer-dropbox/react that referenced this pull request Jul 8, 2016
…help from 1d0c1b1 to get the basic gulp setup (0.14 beta v2 code), and took a lot of dependency wrangling.

`grunt extract-errors` should run the script now.  I'm starting with a fresh codes.json because these will never match up with the official ones unfortunately.

Things skipped:
- travis integration.  I don't know anything about travis so don't know if / how to hook this up for it.
- inserting this as part of a full build process.  It looks like it's a oneoff in master too fwiw
dgoldstein0 pushed a commit to dropbox/react that referenced this pull request Jul 15, 2016
* Remove es3-ify and es5-shim

* adding our own readme

* Update version references

* Rename readme to markdown

* Update the build process to build react-debug.js and react-with-addons-debug.js, which are going to be a hybrid dev/prod build that is faster than the dev version but still has most of the production error messages

* Replace some choice instances of __DEV__ with __DEBUG__, including

- use of factory on React.DOM members
- use of analytics plugin (maybe don't need this...)

Avoided (these are still __DEV__)

- freezing objects
- propType checking
- lots of small checks like warnings of whether style names are valid.

* keep warning messages in debug builds

* Add a hook for registering a callback on warnings

* update dropbox readme with instructions for how to the custom react gets loaded into metaserver

* Fix bug in WarningHandlers

* update README_DROPBOX with info I've learned

* Make warning handlers actually work instead of crash

* This backports bfd1531 from react 15.2 (facebook#6882); it took some help from 1d0c1b1 to get the basic gulp setup (0.14 beta v2 code), and took a lot of dependency wrangling.

`grunt extract-errors` should run the script now.  I'm starting with a fresh codes.json because these will never match up with the official ones unfortunately.

Things skipped:
- travis integration.  I don't know anything about travis so don't know if / how to hook this up for it.
- inserting this as part of a full build process.  It looks like it's a oneoff in master too fwiw

* backport rewriting to error codes working for debug & prod builds from 1abce16

* minor README_DROPBOX tweak

* add space to invariant error message

* Add debug warnings for ReactClass

* Keep some DEV only warnings in DEBUG

* Re-extract error codes

* Include instructions on prod version in dropbox readme

* Fix warning handlers

* Remove instruction to copy over production React.

Instead we should use the official build for prod.

* Revert inclusion of prop mutations to DEV builds

We probably want to include them later on since they're immensely useful for
upgrading to future versions. But for the (at least) initial 0.13.3 we can
live without them.

* Include debug warning about `componentShouldUpdate`

* Update .eslintrc with DEBUG global
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

5 participants