-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
fix(resolve): normalize optimized resolved path #4813
Conversation
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.
Just to be curious, why are all tests passing right now? Do we not have a test case for this?
We did have a test case for this that was added in #4634, under |
a little late but i can confirm that linking to a local vite build with this change makes vite-plugin-svelte's test-suite pass c:\Users\dominikg\develop\sveltejs\vite-plugin-svelte>pnpm test:ci:serve
> vite-plugin-svelte-monorepo@ test:ci:serve c:\Users\dominikg\develop\sveltejs\vite-plugin-svelte
> cross-env VITE_PRESERVE_BUILD_ARTIFACTS=1 jest --verbose --no-cache --runInBand --force-exit --ci --json --outputFile="temp/serve/jest-results.json"
Determining test suites to run...
preparing non ci env...
syncing node_modules
syncing node_modules done
building packages
> vite-plugin-svelte-monorepo@ build:ci c:\Users\dominikg\develop\sveltejs\vite-plugin-svelte
> pnpm --dir packages/vite-plugin-svelte build:ci
> @sveltejs/vite-plugin-svelte@1.0.0-next.19 build:ci C:\Users\dominikg\develop\sveltejs\vite-plugin-svelte\packages\vite-plugin-svelte
> rimraf dist && tsup-node src/index.ts --format esm,cjs --no-splitting
CLI Building entry: src/index.ts
CLI Using tsconfig: C:\Users\dominikg\develop\sveltejs\vite-plugin-svelte\packages\vite-plugin-svelte\tsconfig.json
CLI tsup v4.14.0
CLI Target: node12
ESM Build start
CJS Build start
CJS Build success in 174ms
ESM Build success in 294ms
building packages done
preparations done
PASS packages/e2e-tests/kit-node/__tests__/kit.spec.ts (13.415 s)
kit-node
index route
√ should hydrate (447 ms)
√ should have correct styles applied (15 ms)
√ should increase count on click (82 ms)
√ should not have failed requests
√ should load dynamic import in onMount
hmr
√ should render additional html (719 ms)
√ should render additional child components (453 ms)
√ should apply changed styles (254 ms)
√ should serve changes even after page reload (1014 ms)
child component update
√ should preserve dom order (211 ms)
√ should render additional html (301 ms)
√ should apply changed styles (228 ms)
√ should apply changed initial state (245 ms)
config file update
√ should show an overlay (106 ms)
PASS packages/e2e-tests/hmr/__tests__/hmr.spec.ts (14.221 s)
√ should render App (40 ms)
√ should render static import (3 ms)
√ should render dependency import (16 ms)
√ should render dynamic import (251 ms)
√ should not have failed requests
hmr
√ should have expected initial state (47 ms)
√ should have working increment button (172 ms)
√ should apply css changes in HmrTest.svelte (174 ms)
√ should apply js change in HmrTest.svelte (234 ms)
√ should keep state of external store intact on change of HmrTest.svelte (252 ms)
√ should preserve state of external store used by HmrTest.svelte when editing App.svelte (241 ms)
√ should preserve state of store when editing hmr-stores.js (202 ms)
PASS packages/e2e-tests/vite-ssr/__tests__/vite-ssr.spec.ts (5.599 s)
√ / (35 ms)
√ css (37 ms)
√ asset (15 ms)
hmr
√ should render additional html (332 ms)
√ should apply style update (166 ms)
√ should not preserve state of updated props (373 ms)
PASS packages/e2e-tests/autoprefixer-browerslist/__tests__/autoprefixer-browerslist.spec.ts
√ should prefix position: sticky for code in source tree (34 ms)
PASS packages/e2e-tests/package-json-svelte-field/__tests__/package-json-svelte-field.spec.ts
√ should render component imported via svelte field in package.json (65 ms)
√ should optimize nested deps of excluded svelte deps
PASS packages/vite-plugin-svelte/src/utils/__tests__/dependencies.spec.ts
dependencies
findRootSvelteDependencies
√ should find svelte dependencies in packages/e2e-test/hmr
√ should find nested svelte dependencies in packages/e2e-test/package-json-svelte-field (47 ms)
PASS packages/vite-plugin-svelte/src/utils/__tests__/sourcemap.spec.ts
sourcemap
buildMagicString
√ should return a valid magic string
buildSourceMap
√ should return a map with mappings and filename
PASS packages/e2e-tests/configfile-esm/__tests__/configfile-esm.spec.ts (6.061 s)
√ should load default config and work (68 ms)
√ should load custom cjs config and work (1877 ms)
PASS packages/e2e-tests/configfile-custom/__tests__/configfile-custom.spec.ts (5.214 s)
√ should load default config and work (37 ms)
√ should load custom mjs config and work (1840 ms)
PASS packages/e2e-tests/ts-type-import/__tests__/ts-type-import.spec.ts
√ should render App (22 ms)
hmr
√ should update App (256 ms)
PASS packages/e2e-tests/custom-extensions/__tests__/custom-extensions.spec.ts
√ should render svg (59 ms)
Test Suites: 12 passed, 12 total
Tests: 59 passed, 59 total
Snapshots: 0 total
Time: 71.994 s
Ran all test suites.
Test results written to: temp\serve\jest-results.json
Force exiting Jest: Have you considered using `--detectOpenHandles` to detect async operations that kept running after all tests finished?
c:\Users\dominikg\develop\sveltejs\vite-plugin-svelte> |
Thanks for testing this @dominikg! |
Description
Fix path normalization when trying to resolve an optimized path. This is because the id was normalized here and then passed to here, and when comparing it here,
resolvedSrc
also needs to be normalized.Additional context
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).