Improve plugin-vue output #1310
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/pikapkg/snowpack/4m2tki5kc |
| @@ -108,6 +108,10 @@ module.exports = function plugin(snowpackConfig) { | |||
| if (sourceMaps && js.map) output['.js'].map += JSON.stringify(js.map); | |||
| } | |||
|
|
|||
| // clean up | |||
| if (!output['.js'].code) delete output['.js']; | |||
| if (!output['.css'].code) delete output['.css']; | |||
There was a problem hiding this comment.
This is a very non-controversial change here, that fixes Vue specifically.
I don’t believe other plugins had this problem—this was behavior caused by starting out with a blank code key to start, because multiple Vue things would then run in parallel to speed things up. But after everything ran we failed to check to make sure something was generated in every case (often it wasn’t)
| @@ -84,6 +89,9 @@ async function runPipelineLoadStep( | |||
|
|
|||
| // if source maps disabled, don’t return any | |||
| if (!sourceMaps) result[ext].map = undefined; | |||
|
|
|||
| // clean up empty files | |||
| if (!result[ext].code) delete result[ext]; | |||
There was a problem hiding this comment.
@FredKSchott This, however, is a possibly-controversial change. This asks the question: should we rely on every plugin to clean up perfectly after itself? Or should Snowpack assume an empty file shouldn’t be generated?
If the latter, then maybe this is a good change to make? I don’t see this causing any issues, unless in some cases an empty file isn’t a mistake (though I can’t think of any of those scenarios off the top of my head)
There was a problem hiding this comment.
I thought that today we treated empty string as equivalent to a file that shouldn't be built. I think i saw this issue pop up in some other work that I was doing, so maybe we need to revisit this specifically. it's also different for a plugin that creates more than one file.
There was a problem hiding this comment.
Yeah so we did check that the plugin generated something, but we weren’t checking if it returned { '.js': { code: '' } }. This is the change here.
| @@ -101,10 +109,14 @@ async function runPipelineLoadStep( | |||
| }; | |||
| } | |||
|
|
|||
| async function composeSourceMaps(id: string, base: string | RawSourceMap, derived: string | RawSourceMap) : Promise<string> { | |||
| async function composeSourceMaps( | |||
There was a problem hiding this comment.
Ignore the other changes in this file; these are all just Prettier auto-formats.
drwpow
force-pushed
the
drwpow/improve-plugin-vue
branch
from
2576c5d to
e754143
Compare
| ".css": Object { | ||
| "code": "", | ||
| "map": "", | ||
| }, |
There was a problem hiding this comment.
Hiding right under our noses! 🎉
There was a problem hiding this comment.
aha! Glad we have the individual test files now 👍
Changes
Before, we were sending
.cssfiles to Snowpack from every Vue file, even if the Vue file generated no styles. This PR changes that, and only sends styles to Snowpack when needed.Here are screenshots of a Vue application that loads external styles:
Before (1 empty CSS file generated per-component)
After (no empty CSS files generated)
Basically, all the empty
<style>tags were dozens of empty.css.proxy.jsfiles being generated & loaded.Testing
Tested manually, as seen from screenshots.
But you can also see the snapshot diff in the changes, where some empty CSS files were deleted (comment below)
Docs