-
-
Notifications
You must be signed in to change notification settings - Fork 120
fix build --watch
#1189
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
Merged
Merged
fix build --watch
#1189
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
a2e8363
wip: add test mode for build --watch and e2e setup geared towards it.…
dominikg e00ba38
fix: add missing return
dominikg ea2e4dd
fix: ensure compiled css is loaded correctly when rebuilding in watch…
dominikg efa1b4a
fix: ignore our own warning message in error log test
dominikg File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@sveltejs/vite-plugin-svelte': patch | ||
--- | ||
|
||
fix: ensure compiled svelte css is loaded correctly when rebuilding in `build --watch` |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,3 +52,5 @@ coverage | |
packages/playground/**/* | ||
!packages/playground/README.md | ||
|
||
# vite plugin inspect | ||
.vite-inspect |
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,15 @@ | ||
import path from 'node:path'; | ||
import fs from 'node:fs'; | ||
import MagicString from 'magic-string'; | ||
|
||
function replaceWithSourceMap(code, value, replacement) { | ||
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. the missing sourcemap lead to error logs, add them to allow easier debugging and assertions that no unexpected errors happened |
||
const s = new MagicString(code); | ||
s.replaceAll(value, replacement); | ||
return { | ||
code: s.toString(), | ||
map: s.generateMap({ hires: 'boundary' }) | ||
}; | ||
} | ||
/** | ||
* Ensure transform flow is not interrupted | ||
* @returns {import('vite').Plugin[]} | ||
|
@@ -11,19 +21,19 @@ export function transformValidation() { | |
enforce: 'pre', | ||
transform(code, id) { | ||
if (id.endsWith('.svelte')) { | ||
return code.replaceAll('__JS_TRANSFORM_1__', '__JS_TRANSFORM_2__'); | ||
return replaceWithSourceMap(code, '__JS_TRANSFORM_1__', '__JS_TRANSFORM_2__'); | ||
} else if (id.endsWith('.css')) { | ||
return code.replaceAll('__CSS_TRANSFORM_1__', '__CSS_TRANSFORM_2__'); | ||
return replaceWithSourceMap(code, '__CSS_TRANSFORM_1__', '__CSS_TRANSFORM_2__'); | ||
} | ||
} | ||
}, | ||
{ | ||
name: 'transform-validation:2', | ||
transform(code, id) { | ||
if (id.endsWith('.svelte')) { | ||
return code.replaceAll('__JS_TRANSFORM_2__', '__JS_TRANSFORM_3__'); | ||
return replaceWithSourceMap(code, '__JS_TRANSFORM_2__', '__JS_TRANSFORM_3__'); | ||
} else if (id.endsWith('.css')) { | ||
return code.replaceAll('__CSS_TRANSFORM_2__', 'red'); | ||
return replaceWithSourceMap(code, '__CSS_TRANSFORM_2__', 'red'); | ||
} | ||
} | ||
}, | ||
|
@@ -32,7 +42,7 @@ export function transformValidation() { | |
enforce: 'post', | ||
transform(code, id) { | ||
if (id.endsWith('.svelte')) { | ||
return code.replaceAll('__JS_TRANSFORM_3__', 'Hello world'); | ||
return replaceWithSourceMap(code, '__JS_TRANSFORM_3__', 'Hello world'); | ||
} | ||
// can't handle css here as in build, it would be `export default {}` | ||
} | ||
|
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,5 +6,8 @@ | |
"main": "./index.js", | ||
"files": [ | ||
"index.js" | ||
] | ||
], | ||
"dependencies": { | ||
"magic-string": "^0.30.17" | ||
} | ||
} |
157 changes: 157 additions & 0 deletions
157
packages/e2e-tests/build-watch/__tests__/build-watch.spec.ts
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,157 @@ | ||
import { | ||
isBuildWatch, | ||
getEl, | ||
getText, | ||
editFileAndWaitForBuildWatchComplete, | ||
hmrCount, | ||
untilMatches, | ||
sleep, | ||
getColor, | ||
browserLogs, | ||
e2eServer | ||
} from '~utils'; | ||
|
||
import * as vite from 'vite'; | ||
// @ts-ignore | ||
const isRolldownVite = !!vite.rolldownVersion; | ||
|
||
describe.runIf(isBuildWatch)('build-watch', () => { | ||
test('should render App', async () => { | ||
expect(await getText('#app-header')).toBe('Test-App'); | ||
}); | ||
|
||
test('should render static import', async () => { | ||
expect(await getText('#static-import .label')).toBe('static-import'); | ||
}); | ||
|
||
test('should render dependency import', async () => { | ||
expect(await getText('#dependency-import .label')).toBe('dependency-import'); | ||
}); | ||
|
||
test('should render dynamic import', async () => { | ||
expect(await getEl('#dynamic-import')).toBe(null); | ||
const dynamicImportButton = await getEl('#button-import-dynamic'); | ||
expect(dynamicImportButton).toBeDefined(); | ||
await dynamicImportButton.click(); | ||
await untilMatches( | ||
() => getText('#dynamic-import .label'), | ||
'dynamic-import', | ||
'dynamic import loaded after click' | ||
); | ||
}); | ||
|
||
test('should not have failed requests', async () => { | ||
browserLogs.forEach((msg) => { | ||
expect(msg).not.toMatch('404'); | ||
}); | ||
}); | ||
|
||
test('should respect transforms', async () => { | ||
expect(await getText('#js-transform')).toBe('Hello world'); | ||
expect(await getColor('#css-transform')).toBe('red'); | ||
}); | ||
|
||
describe('edit files', () => { | ||
const updateHmrTest = editFileAndWaitForBuildWatchComplete.bind( | ||
null, | ||
'src/components/HmrTest.svelte' | ||
); | ||
const updateModuleContext = editFileAndWaitForBuildWatchComplete.bind( | ||
null, | ||
'src/components/partial-hmr/ModuleContext.svelte' | ||
); | ||
const updateApp = editFileAndWaitForBuildWatchComplete.bind(null, 'src/App.svelte'); | ||
const updateStore = editFileAndWaitForBuildWatchComplete.bind(null, 'src/stores/hmr-stores.js'); | ||
|
||
const getWatchErrors = () => | ||
isRolldownVite | ||
? e2eServer.logs.watch.err.filter( | ||
(m) => | ||
![ | ||
'Support for rolldown-vite in vite-plugin-svelte is experimental', | ||
'See https://github.com/sveltejs/vite-plugin-svelte/issues/1143' | ||
].some((s) => m.includes(s)) | ||
) | ||
: e2eServer.logs.watch.err; | ||
|
||
test('should have expected initial state', async () => { | ||
// initial state, both counters 0, both labels red | ||
expect(await getText('#hmr-test-1 .counter')).toBe('0'); | ||
expect(await getText('#hmr-test-2 .counter')).toBe('0'); | ||
expect(await getText('#hmr-test-1 .label')).toBe('hmr-test'); | ||
expect(await getText('#hmr-test-2 .label')).toBe('hmr-test'); | ||
expect(await getColor('#hmr-test-1 .label')).toBe('red'); | ||
expect(await getColor('#hmr-test-2 .label')).toBe('red'); | ||
}); | ||
|
||
test('should have working increment button', async () => { | ||
// increment counter of one instance to have local state to verify after build updates | ||
await (await getEl('#hmr-test-1 .increment')).click(); | ||
await sleep(50); | ||
|
||
// counter1 = 1, counter2 = 0 | ||
expect(await getText('#hmr-test-1 .counter')).toBe('1'); | ||
expect(await getText('#hmr-test-2 .counter')).toBe('0'); | ||
}); | ||
|
||
test('should apply css changes in HmrTest.svelte', async () => { | ||
// update style, change label color from red to green | ||
await updateHmrTest((content) => content.replace('color: red', 'color: green')); | ||
|
||
// color should have changed | ||
expect(await getColor('#hmr-test-1 .label')).toBe('green'); | ||
expect(await getColor('#hmr-test-2 .label')).toBe('green'); | ||
expect(getWatchErrors(), 'error log of `build --watch` is not empty').toEqual([]); | ||
}); | ||
|
||
test('should apply js change in HmrTest.svelte ', async () => { | ||
// update script, change label value | ||
await updateHmrTest((content) => | ||
content.replace("const label = 'hmr-test'", "const label = 'hmr-test-updated'") | ||
); | ||
expect(await getText('#hmr-test-1 .label')).toBe('hmr-test-updated'); | ||
expect(await getText('#hmr-test-2 .label')).toBe('hmr-test-updated'); | ||
expect(getWatchErrors(), 'error log of `build --watch` is not empty').toEqual([]); | ||
}); | ||
|
||
test('should reset state of external store used by HmrTest.svelte when editing App.svelte', async () => { | ||
// update App, add a new instance of HmrTest | ||
await updateApp((content) => | ||
content.replace( | ||
'<!-- HMR-TEMPLATE-INJECT -->', | ||
'<HmrTest id="hmr-test-3"/>\n<!-- HMR-TEMPLATE-INJECT -->' | ||
) | ||
); | ||
// counter state is reset | ||
expect(await getText('#hmr-test-1 .counter')).toBe('0'); | ||
expect(await getText('#hmr-test-2 .counter')).toBe('0'); | ||
// a third instance has been added | ||
expect(await getText('#hmr-test-3 .counter')).toBe('0'); | ||
expect(getWatchErrors(), 'error log of `build --watch` is not empty').toEqual([]); | ||
}); | ||
|
||
test('should reset state of store when editing hmr-stores.js', async () => { | ||
// change state | ||
await (await getEl('#hmr-test-2 .increment')).click(); | ||
await sleep(50); | ||
expect(await getText('#hmr-test-2 .counter')).toBe('1'); | ||
await updateStore((content) => `${content}\n/*trigger change*/\n`); | ||
// counter state is reset | ||
expect(await getText('#hmr-test-2 .counter')).toBe('0'); | ||
expect(getWatchErrors(), 'error log of `build --watch` is not empty').toEqual([]); | ||
}); | ||
|
||
test('should work when editing script context="module"', async () => { | ||
expect(await getText('#hmr-with-context')).toContain('x=0 y=1 slot=1'); | ||
expect(await getText('#hmr-without-context')).toContain('x=0 y=1 slot='); | ||
expect(hmrCount('UsingNamed.svelte'), 'updates for UsingNamed.svelte').toBe(0); | ||
expect(hmrCount('UsingDefault.svelte'), 'updates for UsingDefault.svelte').toBe(0); | ||
await updateModuleContext((content) => content.replace('y = 1', 'y = 2')); | ||
expect(await getText('#hmr-with-context')).toContain('x=0 y=2 slot=2'); | ||
expect(await getText('#hmr-without-context')).toContain('x=0 y=2 slot='); | ||
expect(hmrCount('UsingNamed.svelte'), 'updates for UsingNamed.svelte').toBe(0); | ||
expect(hmrCount('UsingDefault.svelte'), 'updates for UsingDefault.svelte').toBe(0); | ||
expect(getWatchErrors(), 'error log of `build --watch` is not empty').toEqual([]); | ||
}); | ||
}); | ||
}); |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
<!doctype html> | ||
<html lang="en"> | ||
<head> | ||
<meta charset="UTF-8" /> | ||
<link rel="icon" href="/favicon.png" /> | ||
<meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
<title>Svelte App</title> | ||
</head> | ||
<body> | ||
<script type="module" src="/src/index.js"></script> | ||
</body> | ||
</html> |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
{ | ||
"name": "e2e-tests-build-watch", | ||
"private": true, | ||
"version": "0.0.0", | ||
"scripts": { | ||
"dev": "vite dev", | ||
"build": "vite build", | ||
"build:watch": "vite build --watch", | ||
"preview": "vite preview" | ||
}, | ||
"dependencies": { | ||
"e2e-test-dep-svelte-simple": "file:../_test_dependencies/svelte-simple" | ||
}, | ||
"devDependencies": { | ||
"@sveltejs/vite-plugin-svelte": "workspace:^", | ||
"e2e-test-dep-vite-plugins": "file:../_test_dependencies/vite-plugins", | ||
"node-fetch": "^3.3.2", | ||
"svelte": "^5.36.13", | ||
"vite": "^7.0.5", | ||
"vite-plugin-inspect": "^11.3.2" | ||
}, | ||
"type": "module" | ||
} |
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
<script> | ||
import StaticImport from './components/StaticImport.svelte'; | ||
import Dependency from 'e2e-test-dep-svelte-simple'; | ||
import HmrTest from './components/HmrTest.svelte'; | ||
import PartialHmr from './components/partial-hmr/PartialHmr.svelte'; | ||
const jsTransform = '__JS_TRANSFORM_1__'; | ||
let dynamicImportComponent; | ||
function importDynamic() { | ||
import('./components/DynamicImport.svelte').then((m) => (dynamicImportComponent = m.default)); | ||
} | ||
</script> | ||
|
||
<h1 id="app-header">Test-App</h1> | ||
<!-- to be transformed into "Hello world!" text --> | ||
<p id="js-transform">{jsTransform}</p> | ||
<!-- to be transformed into "hello-world" class --> | ||
<p id="css-transform">Hello world</p> | ||
<StaticImport /> | ||
<Dependency /> | ||
{#if !dynamicImportComponent} | ||
<button id="button-import-dynamic" on:click={importDynamic}>import dynamic component</button> | ||
{:else} | ||
<svelte:component this={dynamicImportComponent} /> | ||
{/if} | ||
<HmrTest id="hmr-test-1" /> | ||
<HmrTest id="hmr-test-2" /> | ||
|
||
<!-- HMR-TEMPLATE-INJECT --> | ||
|
||
<PartialHmr /> | ||
|
||
<style> | ||
h1 { | ||
color: #111111; | ||
} | ||
|
||
:global(#css-transform) { | ||
color: __CSS_TRANSFORM_1__; | ||
} | ||
</style> |
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
23 changes: 23 additions & 0 deletions
23
packages/e2e-tests/build-watch/src/components/DynamicImport.svelte
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
<script> | ||
import asset from '/src/assets/dynamic.png'; | ||
const importedAsset = asset; | ||
const label = 'dynamic-import'; | ||
</script> | ||
|
||
<div id="dynamic-import"> | ||
<span class="label">{label}</span> <img alt="imported" src={importedAsset} /> | ||
</div> | ||
|
||
<!-- HMR-TEMPLATE-INJECT --> | ||
<style> | ||
.label { | ||
padding-left: 2rem; | ||
background-image: url('/src/assets/dynamic.png'); | ||
background-repeat: no-repeat; | ||
background-size: contain; | ||
} | ||
img { | ||
width: 1rem; | ||
height: 1rem; | ||
} | ||
</style> |
21 changes: 21 additions & 0 deletions
21
packages/e2e-tests/build-watch/src/components/HmrTest.svelte
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
<script> | ||
import { getStore } from '../stores/hmr-stores'; | ||
export let id; | ||
const count = getStore(id, 0); | ||
const label = 'hmr-test'; | ||
function increment() { | ||
$count++; | ||
} | ||
</script> | ||
|
||
<div class="hmr-test" {id}> | ||
<span class="label">{label}</span> | ||
<button class="counter increment" on:click={increment}>{$count}</button> | ||
</div> | ||
|
||
<!-- HMR-TEMPLATE-INJECT --> | ||
<style> | ||
.label { | ||
color: red; | ||
} | ||
</style> |
Oops, something went wrong.
Oops, something went wrong.
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.
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.
build --watch
was not tested at all before but 2 recent issues highlighted that we can't just depend on it being tested by vite itself