Fix sourcemap plugin

[drwpow]

Changes

Was investigating CDN issues and came across skypackjs/skypack-cdn#22. Guy Bedford’s es-module-shims package won’t load because our builtin Rollup plugin catches the following line:

      sourceMappingResolved = `\n//# sourceMappingURL=` + resolveUrl(sourceMapping.slice(21), load.r);

As a result this package couldn’t be installed.

This changes the RegEx to handle end-of-file (single line) differently than middle-of-file

• end of file: if this comment occurs at the end of the file and nothing follows it, remove
• middle of file: this MUST occur on its own line (otherwise it‘s not a comment)

Testing

New snapshot was added for this

Docs

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/pikapkg/snowpack/8yaaahtjl
✅ Preview: https://snowpack-git-drwpow-fix-sourcemap-plugin.pikapkg.vercel.app

[drwpow]

@FredKSchott this now breaks the “mid-file“ rule; maybe we should limit to newline-only instead? I think that’d be muuuch safer

[FredKSchott]

Yea, unfortunately we see this mid-file after bundling multiple files together. That would be a regression (I think lit-html or lit-element was the most popular known example of this).

I know you hate this, but these are such one-off weird cases I'd rather handle this as a one-off solution. How about:

- code.replace(/[^'"`]\/\/+#\s*sourceMappingURL=.+$/gm, '')
+ code.replace(/[^'"`]\/\/+#\s*sourceMappingURL=[^'"`].+$/gm, '')

[drwpow]

What breaks it is the \n too so it‘d have to be:

/[^'"`](\\n)?\/\/+#\s*sourceMappingURL=[^'"`]+$/gm

If it works it works, but I‘m gonna add some tests for this plugin specifically to make sure

[FredKSchott]

👍 sounds good

[drwpow]

Actually, after adding tests, there are more problems with the current RegEx:

[drwpow]

It seems to strip any character that comes before it. Gonna try and get these tests passing, then you can tell me how you feel about the result

[FredKSchott]

Ah yea, in that case you'll need to do:

.replace(/[^'"`]((\\n)?\/\/+#\s*sourceMappingURL=[^'"`]+$)/gm, '$1')
// I think $1 would be correct, but may be $2 due to nested `(\n)`.
// you could turn that nested capture group into a non-capturing group as well

[drwpow]

Updated the RegEx. LMK what you think! And if you have any other cases we need to preserve, happy to add it to the test cases.

[FredKSchott]

ah gotcha, so now youre testing that nothing non-URL comes afterwards in the line? Smart!

only random thing that I can think of: can this line end with a semicolon?

[drwpow]

I don‘t think so? Because it‘s a line comment. And a sourcemap URL probably won‘t have a semicolon in it right?

[FredKSchott]

yea, I don't think so either

[drwpow]

FTR if we need to rewrite this RegEx I really don‘t care about the final structure now that we have tests. Please modify this as-needed (and add tests) if you come across any other cases we need to handle.