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

Don't force sourcemaps on for JSXTransformer APIs #2053

Merged
merged 1 commit into from
Aug 19, 2014

Conversation

zpao
Copy link
Member

@zpao zpao commented Aug 16, 2014

In case somebody doesn't want sourcemaps (eg, react-rails) or r.js

Should fix #972. cc @philix @thomasboyt - Looks like you can stop shipping a modified JSXTransformer (or whatever we end up calling this by the time 0.12 happens - cf #1966)

In case somebody doesn't want sourcemaps (eg, react-rails) or r.js
zpao added a commit that referenced this pull request Aug 19, 2014
Don't force sourcemaps on for JSXTransformer APIs
@zpao zpao merged commit 326be2f into facebook:master Aug 19, 2014
@felipecrv
Copy link
Contributor

@zpao this means less modification of JSXTransformer.js when used with r.js and this is great, however I still have to modify JSXTransformer.js as r.js gets confused by the "use strict" strings [1].

[1] felipecrv/jsx-requirejs-plugin@e682c65

@zpao
Copy link
Member Author

zpao commented Aug 21, 2014

:( That's unfortunate. It actually sounds like an esprima bug because there's not reason it shouldn't be able to parse a string like that.

@zpao
Copy link
Member Author

zpao commented Aug 21, 2014

We should probably have this discussion elsewhere but whatever…

I'm actually a bit surprised about r.js... esprima via npm and r.js (via npm install requirejs, which actually packages the same version of esprima, 1.2.2) can both parse this piece of code. I think it covers the same ground that your changes are working around.

"use strict";
console.log('hello world');
console.log('"use strict";');

var s = '';
if (s !== 'use strict') {
    console.log('use strict');
}

@zpao zpao deleted the jsx-transformer-optional-sourcemaps branch October 30, 2014 01:08
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

Successfully merging this pull request may close these issues.

JSXTransformer in used in r.js optimizer can't load SourceMapGenerator
2 participants