breaking: Css Writer #136
Merged
breaking: Css Writer #136
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- will respect Rollup's `output` naming conventions
- do not emit sourcemap if Rollup told not to - synchronize hashed output filename
benmccann
approved these changes
Oct 11, 2020
| @@ -64,6 +55,12 @@ interface Options { | |||
| // } | |||
| // }, | |||
|
|
|||
| /** | |||
| * Add extra code for development and debugging purposes. | |||
There was a problem hiding this comment.
I think this doesn't really add "extra code", but pretty prints the sourcemap
There was a problem hiding this comment.
For this plugin, yes, but it's forwarded to svelte/compiler :: https://svelte.dev/docs#svelte_compile
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It's really a few fixes, but it since it is a change in the default behavior, safer to just call this breaking.
Doing so means we can also allow #126 to land (re: compatibility concerns) and take this opportunity to finally (!) declare useful peer dependency constraints, including Rollup@2x. (Fixing those can be a separate PR) Edit: #138
Changes
Our CSS Writer is broken. While it does work, it's not entirely useful given its environment. As of today (before this PR), the
CssWriterclass:Completely ignores
output.assetFileNames, forcing"bundle.css"as your final output, even though all other assets/chunks will/are (likely) configured to have hashed filenames.To go back to previous behavior, in their Rollup config, one must set
assetFileNames: "[name].[ext]"or pass a function to customize the output naming format selectively.Completely ignores
output.sourcemapandoutput. sourcemapExcludeSources, which means that we force the emitted CSS files to have sourcemaps, even if we told Rollup to disable sourcemaps outright.To go back to previous behavior, in their Rollup config, one must set
sourcemap: true– sources are included by default.Always prints expanded sourcemaps (aka, multiline JSON). While super minor, there's no reason to go this with production (non-
dev) output.I included tests that ensure all these options are respected. And I made sure that when using hashed asset names (Rollup's default), that the sourcemap and CSS file correctly reference each other's hashed names.