Add source maps #681

drwpow · 2020-07-31T04:51:59Z

Changes

Adds supports for source maps (#666) via the following:

@snowpack/plugin-babel: now Lit Element, Preact, and React all get source map support in build & dev
@snowpack/plugin-svelte: Svelte, too
@snowpack/plugin-vue: …and Vue!
esbuild: with blank templates, esbuild will generate source maps if it can, if another compiler like Babel, Vue, or Svelte isn’t being used

It also adds source map support in the dev server.

Screenshot

Here you can see source maps in the dev server! (this also removes some 404 errors for source maps)

Testing

Some shiny, new .map files were added to the snapshots! 🎉

vercel · 2020-07-31T04:52:04Z

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/my5tolaav
✅ Preview: https://snowpack-git-drwpow-source-maps.pikapkg.vercel.app

packages/snowpack/src/commands/build.ts

FredKSchott

+1 on the interface work and testing improvements, but lets think through the behavior a bit more. I think we'll make fewer mistakes here if think through this starting from the perspective of supporting plugins first, and then adding support for the minification story after that.

packages/snowpack/src/build/build-pipeline.ts

drwpow · 2020-08-04T00:18:38Z

packages/snowpack/src/commands/build.ts

-          allFilesToResolveImports[outLoc] = {baseExt, expandedExt, contents, locOnDisk};
-          break;
+
+      for (const [fileExt, buildResult] of Object.entries(builtFileOutput)) {


⚠️ Big core Change

I’m not sure if we were doing this intentionally or not, but it seemed here we were going against our own design. We weren’t respecting our own build map, it seemed.

When a plugin has multiple outputs (e.g. for Svelte in this PR: .js, .js.map, .css, .css.map), we were only doing one pass for all of those! So at first, adding a .js.map to the output didn’t generate anything at all 😅. The reason we didn’t notice was because we were manually writing .css files, and we were only testing the .js + .css output case before.

This changes this logic to now write all keys of the file, which I think is better. But the question is: will this likely break any legacy plugins? I think not, but wanted to flag it.

ah good catch! Yup, I bet we just grabbed our old logic here without updating it to the new build map format. This should be good to merge, I can't think of what it could be breaking that isn't already broken

drwpow · 2020-08-04T00:27:03Z

packages/snowpack/src/commands/build.ts

-        case '.js': {
-          if (builtFileOutput['.css']) {
-            await fs.mkdir(path.dirname(cssOutPath), {recursive: true});
-            await fs.writeFile(cssOutPath, builtFileOutput['.css'], 'utf-8');


See above comment: we now handle .css in its own pass, so the writeFile below is all that’s needed now. I think the end result is cleaner / easier to follow.

drwpow · 2020-08-04T00:28:50Z

test/create-snowpack-app/snapshots/app-template-svelte-typescript/_dist_/App.css

CSS source maps! 🎉

drwpow · 2020-08-04T00:29:50Z

test/create-snowpack-app/snapshots/app-template-svelte-typescript/_dist_/App.js.map

@@ -0,0 +1 @@
+{"version":3,"names":[],"sources":["App.svelte"],"sourcesContent":["<script lang=\"ts\">\"use strict\";\nlet message = 'Learn Svelte with Typescript';\n</script>\n\n<style>\n:global(body) {\n\tmargin: 0;\n\tfont-family: Arial, Helvetica, sans-serif;\n}\n.App {\n\ttext-align: center;\n}\n.App-header {\n\tbackground-color: #F9F6F6;\n\tcolor: #333;\n\tmin-height: 100vh;\n\tdisplay: flex;\n\tflex-direction: column;\n\talign-items: center;\n\tjustify-content: center;\n\tfont-size: calc(10px + 2vmin);\n}\n.App-link {\n\tcolor: #ff3e00;\n}\n.App-logo {\n\theight: 40vmin;\n\tpointer-events: none;\n\tmargin-bottom: 1.0rem;\n\tanimation: App-logo-spin infinite 1.6s ease-in-out alternate;\n}\n@keyframes App-logo-spin {\n\tfrom {\n\t\ttransform: scale(1);\n\t}\n\tto {\n\t\ttransform: scale(1.06);\n\t}\n}\n</style>\n\n<div class=\"App\">\n\t<header class=\"App-header\">\n\t\t<img src=\"/logo.svg\" class=\"App-logo\" alt=\"logo\" />\n\t\t<p>\n\t\t\tEdit <code>src/App.svelte</code> and save to reload.\n\t\t</p>\n\t\t<a\n\t\t\tclass=\"App-link\"\n\t\t\thref=\"https://svelte.dev\"\n\t\t\ttarget=\"_blank\"\n\t\t\trel=\"noopener noreferrer\"\n\t\t>\n\t\t\t{message}\n\t\t</a>\n\t</header>\n</div>\n"],"mappings":";;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;sBAqDI,OAAO;;;;;;;;;;;;;;;;;;;;;;;;;;;;;IApDP,OAAO,GAAG,8BAA8B;;;CAD1B,YAAY;;;;;;;;;;;"}


JS source maps! 🎉

drwpow · 2020-08-04T00:30:03Z

test/create-snowpack-app/snapshots/app-template-vue/_dist_/App.js.map

@@ -0,0 +1 @@
+{"version":3,"sources":["/Users/drew/Sites/pikapkg/snowpack/packages/@snowpack/app-template-vue/src/App.vue"],"names":["message"],"mappings":";;qBACO,KAAK,EAAC,KAAK;qBACN,KAAK,EAAC,YAAY;gCACxB,aAAmD;EAA9C,GAAG,EAAC,WAAW;EAAC,KAAK,EAAC,UAAU;EAAC,GAAG,EAAC,MAAM;;gCAChD,aAGI;eAPV,iBAIS,QAED;eAAA,aAAwB,cAAlB,aAAW;eANzB,iBAMgC,uBAC1B;;;EAEE,KAAK,EAAC,UAAU;EAChB,IAAI,EAAC,mBAAmB;EACxB,MAAM,EAAC,QAAQ;EACf,GAAG,EAAC,qBAAqB;;;;wBAX/B,aAcM,OAdN,UAcM;IAbJ,aAYS,UAZT,UAYS;MAXP,UAAmD;MACnD,UAGI;MACJ,aAKkB,KALlB,UAKkB,mBAAdA,YAAO","sourcesContent":["\n  <div class=\"App\">\n    <header class=\"App-header\">\n      <img src=\"/logo.svg\" class=\"App-logo\" alt=\"logo\" />\n      <p>\n        Edit\n        <code>src/App.vue</code> and save to reload.\n      </p>\n      <a\n        class=\"App-link\"\n        href=\"https://vuejs.org\"\n        target=\"_blank\"\n        rel=\"noopener noreferrer\"\n      >{{ message }}</a>\n    </header>\n  </div>\n"]}


Vue, too! 🎉

drwpow · 2020-08-04T00:42:27Z

packages/@snowpack/plugin-svelte/README.md

-All plugin `options` are passed directly to the Svelte compiler. See [here](https://svelte.dev/docs#svelte_compile) for a list of supported options.
+| Name         |   Type    | Description                                                       |
+| :----------- | :-------: | :---------------------------------------------------------------- |
+| `sourceMaps` | `boolean` | Set to `false` to disable source map generation (default: `true`) |


Since Svelte + Vue plugins now manage source maps themselves, you can now disable source maps in individual plugins if desired (they’re on by default)

docs/docs/05-configuration.md

packages/snowpack/src/build/build-import-proxy.ts

FredKSchott · 2020-08-04T06:14:46Z

packages/snowpack/src/build/build-pipeline.ts

+          result[ext + '.map'] =
+            typeof output.map === 'object' ? JSON.stringify(output.map) : output.map;
+
+          const sourceMapFile = path


can we turn this into a helper, something like replaceFileExtension?

Also, see https://nodejs.org/api/path.html#path_path_basename_path_ext for a cleaner way to strip the extension

packages/snowpack/src/build/build-pipeline.ts

packages/snowpack/src/commands/build.ts

FredKSchott · 2020-08-04T06:20:44Z

packages/snowpack/src/commands/build.ts

-          allFilesToResolveImports[outLoc] = {baseExt, expandedExt, contents, locOnDisk};
-          break;
+
+      for (const [fileExt, buildResult] of Object.entries(builtFileOutput)) {


ah good catch! Yup, I bet we just grabbed our old logic here without updating it to the new build map format. This should be good to merge, I can't think of what it could be breaking that isn't already broken

packages/snowpack/src/commands/build.ts

packages/snowpack/src/commands/dev.ts

packages/snowpack/src/build/build-pipeline.ts

packages/snowpack/src/commands/dev.ts

drwpow · 2020-08-04T23:47:44Z

packages/snowpack/src/commands/dev.ts

-        });
-        hmrEngine.setEntry(originalReqPath, resolvedImports, isHmrEnabled);
+
+      // transform other files


A little bit of unplanned refactoring, but I had such a tough time finding this code block because it looked different from build.ts. Used a switch statement to organize the file transformations a bit better, and tried to start separating “meta” files (.proxy.js, `.map) from other non-meta files.

FredKSchott

LGTM! With one comment about only writing sourceMap if the option is true.

FredKSchott · 2020-08-04T23:46:32Z

packages/@snowpack/plugin-svelte/plugin.js

        filename: filePath,
      });
+
+      if (!js) return;


not for this PR but related to the logging PR: it would be great to take a pass at our plugins and logging when things like this happen (I think think would be always unexpected).

FredKSchott · 2020-08-05T00:04:47Z

packages/snowpack/src/commands/build.ts

+
+        switch (fileExt) {
+          case '.css': {
+            if (map) code = cssSourceMappingURL(code, sourceMappingURL);


here and others: if sourceMap === false, I wouldn't expect any source maps even if the plugin returned one.

This would simplify things for plugin authors as well, where they can always return map if they'd like and then let Snowpack handle whether it gets written or not. Esp. for plugins like Svelte where the map is always returned.

Let me handle that in the build file step, then, that way it’s only in one place.

Updated in build-pipeline.ts

drwpow · 2020-08-05T00:18:16Z

packages/snowpack/src/build/build-import-proxy.ts

-    return generateCssImportProxy({code, hmr, isDev, config});
+    // if proxying a CSS file, remove its source map (the path no longer applies)
+    const sanitized = code.replace(/\/\*#\s*sourceMappingURL=[^/]+\//gm, '');
+    return expandedExt.endsWith('.module.css')


The more I wrestle with this, the more I start to agree with @FredKSchott where we toss out the idea I tried to introduce of a baseExt (.css) and expandedExt (.module.css). It’s hard knowing where to start. I think going forward I’d like to keep it simple, and always assume we want only the final extension. And handle compound extensions (.module.css, .proxy.js, .js.map) on a case-by-case basis. (edit: internally, I mean. Obviously plugins can handle this already with the resolve.* mapping)

drwpow requested a review from a team as a code owner July 31, 2020 04:51

drwpow commented Jul 31, 2020

View reviewed changes

packages/snowpack/src/commands/build.ts Show resolved Hide resolved

FredKSchott reviewed Jul 31, 2020

View reviewed changes

drwpow force-pushed the drwpow/source-maps branch from 5acc2ad to af8009e Compare July 31, 2020 22:44

vercel bot deployed to Preview July 31, 2020 22:44 View deployment

drwpow force-pushed the drwpow/source-maps branch from af8009e to 9a05d51 Compare August 3, 2020 16:07

vercel bot deployed to Preview August 3, 2020 16:07 View deployment

drwpow force-pushed the drwpow/source-maps branch from 9a05d51 to 6bb105e Compare August 3, 2020 21:23

vercel bot deployed to Preview August 3, 2020 21:23 View deployment

drwpow force-pushed the drwpow/source-maps branch from 6bb105e to df50db6 Compare August 3, 2020 21:46

vercel bot deployed to Preview August 3, 2020 21:46 View deployment

drwpow force-pushed the drwpow/source-maps branch from df50db6 to 41d0bf5 Compare August 3, 2020 23:25

vercel bot deployed to Preview August 3, 2020 23:25 View deployment

drwpow commented Aug 3, 2020

View reviewed changes

packages/snowpack/src/build/build-pipeline.ts Outdated Show resolved Hide resolved

drwpow force-pushed the drwpow/source-maps branch from 41d0bf5 to a654a29 Compare August 4, 2020 00:11

vercel bot deployed to Preview August 4, 2020 00:11 View deployment

drwpow commented Aug 4, 2020

View reviewed changes

drwpow force-pushed the drwpow/source-maps branch from a654a29 to d7bc655 Compare August 4, 2020 00:18

vercel bot deployed to Preview August 4, 2020 00:19 View deployment

drwpow force-pushed the drwpow/source-maps branch from d7bc655 to 0c379ec Compare August 4, 2020 00:24

vercel bot deployed to Preview August 4, 2020 00:24 View deployment

drwpow commented Aug 4, 2020

View reviewed changes

drwpow force-pushed the drwpow/source-maps branch from 0c379ec to d91c04c Compare August 4, 2020 00:38

vercel bot deployed to Preview August 4, 2020 00:39 View deployment

drwpow commented Aug 4, 2020

View reviewed changes

drwpow force-pushed the drwpow/source-maps branch from d91c04c to 38d82dd Compare August 4, 2020 00:42

FredKSchott reviewed Aug 4, 2020

View reviewed changes

FredKSchott mentioned this pull request Aug 4, 2020

Add @snowpack/plugin-postcss-transform #684

Merged

FredKSchott reviewed Aug 4, 2020

View reviewed changes

packages/snowpack/src/commands/dev.ts Outdated Show resolved Hide resolved

drwpow force-pushed the drwpow/source-maps branch from 13f3ac0 to 395174a Compare August 4, 2020 18:20

vercel bot deployed to Preview August 4, 2020 18:20 View deployment

drwpow force-pushed the drwpow/source-maps branch from 395174a to c7008c4 Compare August 4, 2020 18:38

vercel bot deployed to Preview August 4, 2020 18:38 View deployment

drwpow added 2 commits August 4, 2020 14:45

Add source maps

9fcaaa5

Add source maps to Babel plugin

1d8f2eb

drwpow force-pushed the drwpow/source-maps branch from c7008c4 to 6996339 Compare August 4, 2020 20:55

vercel bot deployed to Preview August 4, 2020 20:55 View deployment

drwpow force-pushed the drwpow/source-maps branch from 6996339 to aa8a1f8 Compare August 4, 2020 23:33

vercel bot deployed to Preview August 4, 2020 23:33 View deployment

drwpow force-pushed the drwpow/source-maps branch from aa8a1f8 to c5cc042 Compare August 4, 2020 23:43

vercel bot deployed to Preview August 4, 2020 23:43 View deployment

drwpow commented Aug 4, 2020

View reviewed changes

drwpow force-pushed the drwpow/source-maps branch from c5cc042 to d3bfcb0 Compare August 4, 2020 23:50

vercel bot deployed to Preview August 4, 2020 23:50 View deployment

FredKSchott approved these changes Aug 5, 2020

View reviewed changes

drwpow force-pushed the drwpow/source-maps branch from d3bfcb0 to d8fb47d Compare August 5, 2020 00:14

vercel bot deployed to Preview August 5, 2020 00:14 View deployment

drwpow commented Aug 5, 2020

View reviewed changes

drwpow force-pushed the drwpow/source-maps branch from d8fb47d to 9e40e43 Compare August 5, 2020 00:21

vercel bot deployed to Preview August 5, 2020 00:21 View deployment

PR feedback

9852f7b

drwpow force-pushed the drwpow/source-maps branch from 9e40e43 to 9852f7b Compare August 5, 2020 00:27

vercel bot deployed to Preview August 5, 2020 00:27 View deployment

drwpow merged commit c9348d4 into master Aug 5, 2020

drwpow deleted the drwpow/source-maps branch August 5, 2020 00:39

drwpow mentioned this pull request Aug 5, 2020

sourcemap support #666

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add source maps #681

Add source maps #681

drwpow commented Jul 31, 2020 •

edited

vercel bot commented Jul 31, 2020 •

edited

FredKSchott left a comment

drwpow Aug 4, 2020 •

edited

FredKSchott Aug 4, 2020

drwpow Aug 4, 2020 •

edited

drwpow Aug 4, 2020

drwpow Aug 4, 2020

drwpow Aug 4, 2020

drwpow Aug 4, 2020

FredKSchott Aug 4, 2020

FredKSchott Aug 4, 2020

drwpow Aug 4, 2020

FredKSchott left a comment

FredKSchott Aug 4, 2020

FredKSchott Aug 5, 2020

drwpow Aug 5, 2020

drwpow Aug 5, 2020

drwpow Aug 5, 2020 •

edited

		@@ -0,0 +1 @@
		{"version":3,"names":[],"sources":["App.svelte"],"sourcesContent":["<script lang=\"ts\">\"use strict\";\nlet message = 'Learn Svelte with Typescript';\n</script>\n\n<style>\n:global(body) {\n\tmargin: 0;\n\tfont-family: Arial, Helvetica, sans-serif;\n}\n.App {\n\ttext-align: center;\n}\n.App-header {\n\tbackground-color: #F9F6F6;\n\tcolor: #333;\n\tmin-height: 100vh;\n\tdisplay: flex;\n\tflex-direction: column;\n\talign-items: center;\n\tjustify-content: center;\n\tfont-size: calc(10px + 2vmin);\n}\n.App-link {\n\tcolor: #ff3e00;\n}\n.App-logo {\n\theight: 40vmin;\n\tpointer-events: none;\n\tmargin-bottom: 1.0rem;\n\tanimation: App-logo-spin infinite 1.6s ease-in-out alternate;\n}\n@keyframes App-logo-spin {\n\tfrom {\n\t\ttransform: scale(1);\n\t}\n\tto {\n\t\ttransform: scale(1.06);\n\t}\n}\n</style>\n\n<div class=\"App\">\n\t<header class=\"App-header\">\n\t\t<img src=\"/logo.svg\" class=\"App-logo\" alt=\"logo\" />\n\t\t<p>\n\t\t\tEdit <code>src/App.svelte</code> and save to reload.\n\t\t</p>\n\t\t<a\n\t\t\tclass=\"App-link\"\n\t\t\thref=\"https://svelte.dev\"\n\t\t\ttarget=\"_blank\"\n\t\t\trel=\"noopener noreferrer\"\n\t\t>\n\t\t\t{message}\n\t\t</a>\n\t</header>\n</div>\n"],"mappings":";;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;sBAqDI,OAAO;;;;;;;;;;;;;;;;;;;;;;;;;;;;;IApDP,OAAO,GAAG,8BAA8B;;;CAD1B,YAAY;;;;;;;;;;;"}

		@@ -0,0 +1 @@
		{"version":3,"sources":["/Users/drew/Sites/pikapkg/snowpack/packages/@snowpack/app-template-vue/src/App.vue"],"names":["message"],"mappings":";;qBACO,KAAK,EAAC,KAAK;qBACN,KAAK,EAAC,YAAY;gCACxB,aAAmD;EAA9C,GAAG,EAAC,WAAW;EAAC,KAAK,EAAC,UAAU;EAAC,GAAG,EAAC,MAAM;;gCAChD,aAGI;eAPV,iBAIS,QAED;eAAA,aAAwB,cAAlB,aAAW;eANzB,iBAMgC,uBAC1B;;;EAEE,KAAK,EAAC,UAAU;EAChB,IAAI,EAAC,mBAAmB;EACxB,MAAM,EAAC,QAAQ;EACf,GAAG,EAAC,qBAAqB;;;;wBAX/B,aAcM,OAdN,UAcM;IAbJ,aAYS,UAZT,UAYS;MAXP,UAAmD;MACnD,UAGI;MACJ,aAKkB,KALlB,UAKkB,mBAAdA,YAAO","sourcesContent":["\n <div class=\"App\">\n <header class=\"App-header\">\n <img src=\"/logo.svg\" class=\"App-logo\" alt=\"logo\" />\n <p>\n Edit\n <code>src/App.vue</code> and save to reload.\n </p>\n <a\n class=\"App-link\"\n href=\"https://vuejs.org\"\n target=\"_blank\"\n rel=\"noopener noreferrer\"\n >{{ message }}</a>\n </header>\n </div>\n"]}

Add source maps #681

Add source maps #681

Conversation

drwpow commented Jul 31, 2020 • edited

Changes

Testing

vercel bot commented Jul 31, 2020 • edited

FredKSchott left a comment

Choose a reason for hiding this comment

drwpow Aug 4, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drwpow Aug 4, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FredKSchott left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drwpow Aug 5, 2020 • edited

Choose a reason for hiding this comment

drwpow commented Jul 31, 2020 •

edited

vercel bot commented Jul 31, 2020 •

edited

drwpow Aug 4, 2020 •

edited

drwpow Aug 4, 2020 •

edited

drwpow Aug 5, 2020 •

edited