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 JSON string source maps returned from rollup plugins #783

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Aeriit
Copy link

@Aeriit Aeriit commented Jul 27, 2021

Some rollup plugins (such as rollup-plugin-esbuild) return the source map as a string instead of an object; this string needs to be parsed before we can normalize the source URLs (because right now it throws a "cannot read property 'map' of undefined").

@changeset-bot
Copy link

changeset-bot bot commented Jul 27, 2021

🦋 Changeset detected

Latest commit: c02209b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
wmr Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Member

@marvinhagemeister marvinhagemeister left a comment

Choose a reason for hiding this comment

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

Looking good! Let's add a test case for this so we don't run the risk of regressing on this behavior here.

For that you can create a new sample project in the fixture directory and add a simple noop plugin, that returns a string for the map property. There are a few examples on how to run those fixtures from our test suite in ´fixtures.test.js`

try {
result.map = JSON.parse(result.map);
} catch {
if (hasDebugFlag()) {
Copy link
Member

Choose a reason for hiding this comment

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

No need to wrap this with hasDebugFlag(). That check is already part of logTransform.

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.

2 participants