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

Single-pass transformation #52

Merged
merged 1 commit into from
Nov 27, 2016
Merged

Single-pass transformation #52

merged 1 commit into from
Nov 27, 2016

Conversation

ariya
Copy link
Contributor

@ariya ariya commented Oct 27, 2016

This is utilizing Esprima's syntax delegate feature to track directly each syntax node as it is being constructed, looking for process.env.SOMETHING or process.env["SOMETHING"], and record the individual source replacement into an array (the whole replacements are applied directly on the source input later on). Thus, there is no need for an additional pass to traverse the syntax tree.

There is no API change, all tests pass.

@ariya
Copy link
Contributor Author

ariya commented Oct 27, 2016

Note that this change removes the need to use jstransform (which has been deprecated and which, IMHO, is a bit overkill for this kind of source transformation) and it also likely supersedes PR #30.

@jesstelford
Copy link

This fork fixed the following error for me when using rollup + rollup-plugin-browserify-transform:

Error transforming /home/test/node_modules/styled-components/dist/styled-components.es.js: Parse Error: Line 1: Illegal import declaration

@ariya
Copy link
Contributor Author

ariya commented Nov 26, 2016

@hughsk @zertosh Any interest in this PR? If yes, please advise on how to move forward 😸 . Thanks!

@hughsk hughsk merged commit 79d00c8 into hughsk:master Nov 27, 2016
@hughsk
Copy link
Owner

hughsk commented Nov 27, 2016

Hey @ariya! Thanks very much, sorry for the delayed response — looks good to me :D

@jaredbeck
Copy link

Given that this was the only change between 3.4.1 and 4.0.0, and Ariya describes it as "no API change", why was the major version number incremented? To indicate the change in dependencies? If there really were no breaking changes in the API, it seems to me that the major version number should not be incremented. Of course, semver allows an increment, and it's your project Hugh, I just found it confusing.

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.

None yet

4 participants