Fix problem #960
Fix problem #960
Conversation
1. The nameCache obtained in writebundle hook is not a reference to terseroptions 2. JSON.stringify(nameCache) requires converting regex to a string first.
🦋 Changeset detectedLatest commit: b0f2291 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
Would you mind splitting half of this out into another PR? Fixing two (seemingly) different issues in one PR makes it a bit hard to review and follow. Edit: Also helps the history. With both fixes in one PR, it's a bit murky which bit of code fixes what issue. If there's ever problems, it helps if it's clear to what issue a line was altered/added. For both, do you mind writing up issues showcasing the problems you're experiencing?
This is a bit vague, for example. It's difficult to review the work because I don't know what issues you've run into or if your added test cases adequately check your fix. Thanks |
What should I do to split into two pr
|
CSS fixes in one, Terser thing in another. Specific commits get lost as PRs are squashed and merged. |
| basic-css.css | ||
| basic-css.css.map | ||
| basic-css.esm.mjs | ||
| basic-css.esm.mjs.map | ||
| basic-css.js | ||
| basic-css.js.map | ||
| basic-css.umd.js | ||
| basic-css.umd.js.map | ||
| index.css | ||
| index.css.map |
There was a problem hiding this comment.
This shouldn't be a patch if file names are changing without any user behavior alterations.
In spirit of keeping naming consistent, CSS output should also match every other file output by default, not fallback to index IMO.
Can we revert this bit, or switch this to a minor please?
| "regex": "^_" | ||
| "regex": "^_", | ||
| "reserved": [] |
There was a problem hiding this comment.
Did these mangle files need to be updated in order to ensure tests still pass? If so, this too is a breaking change and the changeset should be corrected to mark this as a minor (Semver 0-major, breaking change = minor bump).
| @@ -333,6 +328,7 @@ function getMain({ options, entry, format }) { | |||
| const shebang = {}; | |||
|
|
|||
| function createConfig(options, entry, format, writeMeta) { | |||
| const entryFileName = basename(entry, extname(entry)); | |||
There was a problem hiding this comment.
This is incorrect. Please use the existing naming setup that we had, it ensures names are properly stripped:
Line 505 in 9a4e2b2
index.esm.js as an input needs to become index; yours would have it become index.esm, so index.esm.css would be output. We need to strip indicators towards ES module format, i.e., cjs, es[m], etc.
| options.output.replace(EXTENSION, '.css'), | ||
| resolve(outputDir, `${entryFileName}.css`), |
There was a problem hiding this comment.
How about changing it to this
I have split it into two independent PRs and modified them. Can you make some suggestions or reply to the changes |
fix: css compilation problems when multiple entries
fix: Fix problems related to use rollup-plugin-terser