Update browser runtime #409
Conversation
🦋 Changeset detectedLatest commit: 8b18442 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
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 |
| @@ -15,7 +15,8 @@ | |||
| "author": "SEEK", | |||
| "license": "MIT", | |||
| "dependencies": { | |||
| "@vanilla-extract/integration": "^1.4.2" | |||
| "@vanilla-extract/integration": "^1.4.2", | |||
| "outdent": "^0.8.0" | |||
There was a problem hiding this comment.
Realised we're already using dedent in the css package so we should probably be consistent.
packages/integration/package.json
Outdated
| @@ -3,6 +3,7 @@ | |||
| "version": "1.4.2", | |||
| "description": "Zero-runtime Stylesheets-in-TypeScript", | |||
| "main": "dist/vanilla-extract-integration.cjs.js", | |||
| "module": "dist/vanilla-extract-integration.esm.js", | |||
There was a problem hiding this comment.
This isn't required anymore.
| processVanillaFile, | ||
| parseFileScope, | ||
| stringifyFileScope, | ||
| } from './processVanillaFile'; |
There was a problem hiding this comment.
We need to add a changeset for the integration package
packages/vite-plugin/src/index.ts
Outdated
| if (extensionIndex > 0) { | ||
| const fileScopeId = id.substring(0, extensionIndex); | ||
|
|
||
| if (cssMap.has(fileScopeId)) { |
There was a problem hiding this comment.
Wondering if we should throw an explicit error if this condition is false. It should never really occur yeah?
There was a problem hiding this comment.
Yeah that makes sense, I don't think it should be false ever.
packages/vite-plugin/src/index.ts
Outdated
|
|
||
| if (ssr || useRuntime) { | ||
| if (ssr) { | ||
| const validId = id.substring(0, id.indexOf('?')); |
There was a problem hiding this comment.
This needs to be resilient to strings that don't have a query attached.
packages/vite-plugin/src/index.ts
Outdated
| packageInfo, | ||
| }).source; | ||
| } | ||
|
|
||
| const { source, watchFiles } = await compile({ | ||
| filePath: fixedId, | ||
| filePath: id, |
There was a problem hiding this comment.
I think we should continue to strip the query if it has one, just to be sure there's no funny business.
| cwd: config.root, | ||
| }); | ||
|
|
||
| for (const file of watchFiles) { | ||
| this.addWatchFile(file); | ||
| if (config.command === 'build' || file !== id) { |
There was a problem hiding this comment.
Might be worth adding a comment as to why we have the config.command === 'build' check.
packages/vite-plugin/src/index.ts
Outdated
| } | ||
|
|
||
| return processVanillaFile({ | ||
| source, | ||
| filePath: fixedId, | ||
| outputCss: !ssr, | ||
| filePath: id, |
There was a problem hiding this comment.
Think we should continue to strip the query here as well.
|
|
||
| injectStyles({ | ||
| fileScope: ${JSON.stringify(fileScope)}, | ||
| css: ${JSON.stringify(css)} |
There was a problem hiding this comment.
This seems to be borking the screenshot tests due to the newline encoding
Primarily moving to the new vanilla runtime.
Also includes a bunch of bugfixes especially around the vite plugin.