-
Notifications
You must be signed in to change notification settings - Fork 298
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support React Server Components in Vite plugin and fix HMR #751
Changes from 5 commits
0bc2b9f
13718d3
05e2974
544ecde
ee57d8e
2f0a87b
9bb1661
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@vanilla-extract/vite-plugin': patch | ||
--- | ||
|
||
Fix HMR and support React Server Components. | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -68,29 +68,33 @@ export function vanillaExtractPlugin({ | |
virtualExt = `.vanilla.${config.command === 'serve' ? 'js' : 'css'}`; | ||
}, | ||
resolveId(source) { | ||
if (!source.endsWith(virtualExt)) { | ||
const [validId, query] = source.split('?'); | ||
if (!validId.endsWith(virtualExt)) { | ||
return; | ||
} | ||
|
||
// Absolute paths seem to occur often in monorepos, where files are | ||
// imported from outside the config root. | ||
const absoluteId = source.startsWith(config.root) | ||
? source | ||
: getAbsoluteVirtualFileId(source); | ||
: getAbsoluteVirtualFileId(validId); | ||
|
||
// There should always be an entry in the `cssMap` here. | ||
// The only valid scenario for a missing one is if someone had written | ||
// a file in their app using the .vanilla.js/.vanilla.css extension | ||
if (cssMap.has(absoluteId)) { | ||
return absoluteId; | ||
// Keep the original query string for HMR. | ||
return absoluteId + (query ? `?${query}` : ''); | ||
} | ||
}, | ||
load(id) { | ||
if (!cssMap.has(id)) { | ||
const [validId] = id.split('?'); | ||
|
||
if (!cssMap.has(validId)) { | ||
return; | ||
} | ||
|
||
const css = cssMap.get(id); | ||
const css = cssMap.get(validId); | ||
|
||
if (typeof css !== 'string') { | ||
return; | ||
|
@@ -102,21 +106,23 @@ export function vanillaExtractPlugin({ | |
|
||
return outdent` | ||
import { injectStyles } from '@vanilla-extract/css/injectStyles'; | ||
|
||
const inject = (css) => injectStyles({ | ||
fileScope: ${JSON.stringify({ filePath: id })}, | ||
fileScope: ${JSON.stringify({ filePath: validId })}, | ||
css | ||
}); | ||
|
||
inject(${JSON.stringify(css)}); | ||
|
||
import.meta.hot.on('${styleUpdateEvent(id)}', (css) => { | ||
import.meta.hot.on('${styleUpdateEvent(validId)}', (css) => { | ||
inject(css); | ||
}); | ||
}); | ||
`; | ||
}, | ||
async transform(code, id, ssrParam) { | ||
if (!cssFileFilter.test(id)) { | ||
const [validId] = id.split('?'); | ||
|
||
if (!cssFileFilter.test(validId)) { | ||
return null; | ||
} | ||
|
||
|
@@ -128,10 +134,7 @@ export function vanillaExtractPlugin({ | |
ssr = ssrParam?.ssr; | ||
} | ||
|
||
const index = id.indexOf('?'); | ||
const validId = index === -1 ? id : id.substring(0, index); | ||
|
||
if (ssr) { | ||
if (ssr && !process.env.VITE_RSC_BUILD) { | ||
return addFileScope({ | ||
source: code, | ||
filePath: normalizePath(validId), | ||
|
@@ -149,7 +152,7 @@ export function vanillaExtractPlugin({ | |
for (const file of watchFiles) { | ||
// In start mode, we need to prevent the file from rewatching itself. | ||
// If it's a `build --watch`, it needs to watch everything. | ||
if (config.command === 'build' || file !== id) { | ||
if (config.command === 'build' || file !== validId) { | ||
this.addWatchFile(file); | ||
} | ||
} | ||
|
@@ -183,10 +186,16 @@ export function vanillaExtractPlugin({ | |
cssMap.get(absoluteId) !== source | ||
) { | ||
const { moduleGraph } = server; | ||
const module = moduleGraph.getModuleById(absoluteId); | ||
const [module] = Array.from( | ||
moduleGraph.getModulesByFile(absoluteId) || [], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not saying this is wrong, just for my own knowledge would like to understand this change more? Assuming the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That's right, according to my tests. A "module id" consists of a file path plus an optional query string that Vite adds for HMR or for other reasons. However, our |
||
); | ||
|
||
if (module) { | ||
moduleGraph.invalidateModule(module); | ||
|
||
// Vite uses this timestamp to add `?t=` query string automatically for HMR. | ||
module.lastHMRTimestamp = | ||
(module as any).lastInvalidationTimestamp || Date.now(); | ||
} | ||
|
||
server.ws.send({ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please split this into two changesets?
patch
release:Fix HMR
minor
release:Add [experimental support for React Server Components](https://github.com/facebook/react/pull/22952)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done 👍
If you want I can split the PRs as well (extract the env variable in a new PR).
Originally, I did it in one PR because at first I thought the HMR was only broken for RSC 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As long as it's two changesets, I'm happy 😊