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

Fix sourceMappingURL for bundles with multiple entry points [CSS] #2867

Closed
wants to merge 7 commits into from
Closed

Fix sourceMappingURL for bundles with multiple entry points [CSS] #2867

wants to merge 7 commits into from

Conversation

rynpsc
Copy link

@rynpsc rynpsc commented Mar 28, 2019

↪️ Pull Request

Fixes incorrect sourcemap URL's when you have multiple entry points.

Related issue #2851

💻 Examples

Given the followong CLI command

parcel build src/a/style.css src/b/style.css

The following files are produced

dist/a/style.css /*# sourceMappingURL=/style.css.map */
dist/b/style.css /*# sourceMappingURL=/style.css.map */

The sourcemap URLs are incorrect they should be /a/style.css.map and /b/style.css.map.

With the fix in place we instead get:

dist/a/style.css /*# sourceMappingURL=/a/style.css.map */
dist/b/style.css /*# sourceMappingURL=/b/style.css.map */

The same issue was previously fixed with JavaScript sourcemaps #2645

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

@rynpsc rynpsc marked this pull request as ready for review March 30, 2019 08:59
@rynpsc
Copy link
Author

rynpsc commented Mar 30, 2019

Have updated desciptions and added tests. Currently failing due to #2842.

@rynpsc rynpsc changed the title Ensure CSS sourceMappingURL is relative Fix sourceMappingURL for bundles with multiple entry points [CSS] Mar 30, 2019
mischnic
mischnic previously approved these changes Apr 4, 2019
@mischnic
Copy link
Member

mischnic commented Apr 4, 2019

Now consistent with the logic in JSPackager.js 👍

@rynpsc
Copy link
Author

rynpsc commented Apr 4, 2019

@mischnic Noticed the check for the sourcemap comment was using JS syntax rather than CSS.

@mischnic
Copy link
Member

mischnic commented Apr 4, 2019

Noticed the check for the sourcemap comment was using JS syntax rather than CSS.

We really need to get CI working again.

Very picky comment: could you rename the vars in that test? (from jsOutput)

mischnic
mischnic previously approved these changes Apr 4, 2019
@sammckay10
Copy link

Has this been merged?!?

@mmcc
Copy link

mmcc commented Nov 20, 2019

I'm running into this exact issue and it appears this was approved, but closed instead of merged (unless I'm mistaken). Was that an accident?

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

6 participants