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

Handle null mappings correctly in sourcemaps #1851

Closed
wants to merge 639 commits into from

Conversation

DeMoorJasper
Copy link
Member

@DeMoorJasper DeMoorJasper commented Aug 3, 2018

This allows null mappings in parcels sourcemaps.
Creating more accurate sourcemaps.
Also adds a couple extra tests for sourcemaps, using the SourceMap class directly, giving a more in depth coverage of SourceMap

Closes #1825

cc @loganfsmyth

olizilla and others added 30 commits February 4, 2018 20:12
* always add the bundle-loader module

* check if the asset is already in the bundle

* make sure bundles is an array in loadBundlesLazy
* support no-config jsx

* Automatically enable JSX in normal .js files if react or preact are dependencies
Put a `require` call inside a try…catch block, and it becomes optional. If that dependency cannot be resolved, it will throw a runtime error instead of a build time one.
* Run the tests on node 6
* Disable web assembly tests if not supported
* Check if we should ignore a babelrc before merging
For node, specifying `>= 6` is wrong, it should say just `6`
Removed call to window.location
* add server reference to bundler

* remove let
devongovett and others added 21 commits July 15, 2018 18:49
Should bring a noticeable size improvement on big TypeScript apps
Now that Parcel is using Spectrum instead of Slack for community management, I figured we should update `CONTRIBUTING.md` accordingly.

Happy to make any language changes you feel are necessary!
Fixes browser errors thrown due to multiple certificates using the same issuer and serial number.  Occurs when a self-signed certificate has already been stored in the browser and new certificate is generated; either in the same project or across project installations.
Update to typescript 3.0.0, it's just so the tests run the latest version.
There appears to be no breaking changes.
Just a small documentation change -- as of #1040, the default value for `--public-url` is `/`, not the `--out-dir` option anymore.
map = typeof map === 'string' ? JSON.parse(map) : map;
if (map.sourceRoot) delete map.sourceRoot;

Choose a reason for hiding this comment

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

Is the intention with this to normalize the file paths somehow? It may well make sense for Parcel to normalize this stuff somehow, but dropping it entirely seems like it could lose useful information.

Copy link
Member Author

Choose a reason for hiding this comment

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

The mapConsumer resolves paths if the sourcemap has a sourceRoot (turning them into absolute paths), so if parcel would be consuming a map this would duplicate the mappings, or make them no longer relative to the new sourceRoot (as internally parcel and sourcemaps use relative paths).

I'm not sure how else to solve this, I am pretty positive that this will rarely/never cause issues inside parcel as the source is defined where parcel inits the sourcemap (for example JSAsset).

EDIT: This might cause issues with sourcemaps we import from existing modules. If they have filenames that overlap with one used inside the sourcecode of the project

@devongovett
Copy link
Member

Due to move to monorepo which rewrote the git history, this PR was automatically closed. Please open a new PR against master with your changes. You can do a diff against the 1.x branch to see them. Sorry about that!

@DeMoorJasper DeMoorJasper deleted the fix/sourcemap-null-mappings branch October 16, 2018 06:44
@DeMoorJasper DeMoorJasper restored the fix/sourcemap-null-mappings branch October 16, 2018 06:44
@DeMoorJasper DeMoorJasper deleted the fix/sourcemap-null-mappings branch January 11, 2019 19:07
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