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

Source-map support for multiple input source files #3323

Merged
merged 4 commits into from
Mar 7, 2016
Merged

Source-map support for multiple input source files #3323

merged 4 commits into from
Mar 7, 2016

Conversation

divmain
Copy link
Contributor

@divmain divmain commented Feb 5, 2016

This PR allows tools built on top of Babel to parse multiple JS sources to AST, optionally transform them, and combine them. When output code is generated, source-maps will point to the original source files.

The minimum change necessary for my use case is contained in the first commit. However, the additional changes add support for this use case to both ends of the Babel toolchain (and removes unnecessary traversals of the AST outside of Babel).

Documentation was added where it seemed it would be helpful. I did not add tests since the test helpers assume a 1:1 transformation. If you'd like me to add tests, let me know what it should look like.

Thanks!

@codecov-io
Copy link

Current coverage is 87.61%

Merging #3323 into master will decrease coverage by -0.04% as of dd2e54d

@@            master   #3323   diff @@
======================================
  Files          217     217       
  Stmts        15238   15246     +8
  Branches      3120    3124     +4
  Methods          0       0       
======================================
+ Hit          13357   13358     +1
- Partial        614     622     +8
+ Missed        1267    1266     -1

Review entire Coverage Diff as of dd2e54d

Powered by Codecov. Updated on successful CI builds.

@hzoo hzoo added the PR: New Feature 🚀 A type of pull request used for our changelog categories label Feb 5, 2016
@amasad
Copy link
Member

amasad commented Mar 4, 2016

Why not concat the files after the transform as opposed to before transform? And then combine the sourcemaps. I know that using the combine-source-map module is awfully slow but you can use this neat trick:

https://github.com/facebook/react-native/blob/ea882b6f161409ff439b93dfa146431944a42d9d/packager/react-packager/src/Bundler/Bundle.js#L199-L232

It didn't have full browser support last I checked, but let me know if that solves it for you

@divmain
Copy link
Contributor Author

divmain commented Mar 4, 2016

Hey @amasad, thanks for the link. That's an interesting work-around. I'm not sure it will work for me - I'm building a bundler similar in function to webpack, and losing broad browser support would be a significant drawback.

The change proposed here is really the only thing keeping Babel from being used in place of something like recast. And it is also something that escodegen supports - I've been using that until recently for code/map generation, but would really prefer to use Babel for everything. When we discussed awhile back in the Slack channel, it seemed like there was an openness to the change.

There's definitely risk in opening Babel to new use-cases like this. And I also recognize that there may not be a lot in it for the Babel maintainers :) But if there's anything I can do to make it a more attractive change, I'm definitely open. Since it is a minimal and non-breaking change, I hope that there's a possibility.

@@ -6,11 +6,12 @@ import { SourceLocation } from "../util/location";
const pp = Parser.prototype;

class Node {
constructor(pos?: number, loc?: SourceLocation) {
constructor(pos?: number, loc?: SourceLocation, filename?: any) {
Copy link
Member

Choose a reason for hiding this comment

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

filename?: string

@amasad
Copy link
Member

amasad commented Mar 4, 2016

OK. Sounds good. Can you please add tests?

amasad added a commit that referenced this pull request Mar 7, 2016
Source-map support for multiple input source files
@amasad amasad merged commit 4b2a660 into babel:master Mar 7, 2016
JacopKane pushed a commit to JacopKane/babel that referenced this pull request Jan 11, 2018
Source-map support for multiple input source files
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 7, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: New Feature 🚀 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants