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

Remove leading slash from paths in Babel options #8610

Merged
merged 1 commit into from May 3, 2017

Conversation

@klaussner
Copy link
Member

@klaussner klaussner commented Apr 17, 2017

Removes the leading slash from the filename and sourceFileName Babel options so that Babel plugins can distinguish relative paths from absolute paths.

I couldn't find out if the filename property is used somewhere in the Meteor build process but the sourceFileName is used in tools/isobuild/bundler.js where the removed slash is handled correctly.

Fixes #8566

@benjamn
Copy link
Member

@benjamn benjamn commented Apr 19, 2017

Do these paths get used in the source maps generated by Babel? I wonder if absolute vs. relative paths could be an issue there?

@klaussner
Copy link
Member Author

@klaussner klaussner commented Apr 20, 2017

Yes, the sourceFileName is the path that will end up in the sources array of the source map, so that one must be relative.

@benjamn
Copy link
Member

@benjamn benjamn commented Apr 24, 2017

I vaguely remember a bug when loading a source map on a page like https://example.com/nested/query/string. In the Dev Tools, paths without leading /s get interpreted relatively, e.g. packages/whatever.js would be displayed as /nested/query/string/packages/whatever.js, which is a little confusing.

Maybe we can just modify the paths in sources to add back the leading /s?

@klaussner
Copy link
Member Author

@klaussner klaussner commented Apr 26, 2017

I think the source maps generated by BabelCompiler are never served directly to the client (or used for server-side debugging). Instead, meteor://💻app is prepended to the sourceFileName and the result is always meteor://💻app/packages/whatever.js, regardless of whether there is a leading /.

@benjamn benjamn merged commit f0223c9 into meteor:devel May 3, 2017
3 checks passed
3 checks passed
CLA Author has signed the Meteor CLA.
Details
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@benjamn benjamn added this to the Release 1.5 milestone May 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants