chore: use correct Vite instance for runner checks - #4
anurag6569201 wants to merge 1 commit into
Conversation
Source PR: sveltejs#16973 Source head: 82d2454
⛔ Shipwright · BlockedRecommendation: do not merge PR #4 · Tier
Findings (4)
Fireworks usage: 7,093 input · 549 output · 7,642 total tokens · $0.0019 · 9s · 0 fix iteration(s) Open the Shipwright check for full evidence and the audit bundle. Use |
![shipwright-agent[bot]](https://avatars.githubusercontent.com/u/140162738?s=60&v=4){kind=small}
| config: { | ||
| order: 'pre', | ||
| async handler(config, config_env) { | ||
| handler(config, config_env) { |
There was a problem hiding this comment.
Shipwright · CRITICAL
The 'config.handler' was changed from 'async' to synchronous, but it still performs asynchronous work via 'resolve_entry' and 'get_import_aliases'.
Impact: The 'config.handler' was changed from 'async' to synchronous, but it still performs asynchronous work via 'resolve_entry' and 'get_import_aliases'. If any of these paths rely on async resolution (e.g., dynamic imports, file system promises), the handler will return before the work completes, causing 'normalized_aliases' to be undefined or stale during the build. This can break alias resolution and produce incorrect…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| * @param {ViteDevServer} server | ||
| */ | ||
| export function get_runner(vite, server) { | ||
| export function get_runner({ isRunnableDevEnvironment }, server) { |
There was a problem hiding this comment.
Shipwright · CRITICAL
'get_runner' now destructures 'isRunnableDevEnvironment' from the first argument, but the call site in 'index.js' still passes '() => ({ root, vite })' to 'plugin_remote'.
Impact: 'get_runner' now destructures 'isRunnableDevEnvironment' from the first argument, but the call site in 'index.js' still passes '() => ({ root, vite })' to 'plugin_remote'. If 'plugin_remote' invokes 'get_runner' with the old shape (a full vite module object), the destructured property will be 'undefined', causing a TypeError when calling 'isRunnableDevEnvironment(server.environments.ssr)'. This breaks SSR dev ser…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| import path from 'node:path'; | ||
| import process from 'node:process'; | ||
| import { styleText } from 'node:util'; | ||
| import * as vite from 'vite'; |
There was a problem hiding this comment.
Shipwright · HIGH
The import of '* as vite from 'vite'' at the top level of 'index.js' changes the module loading strategy from a peer-resolved dynamic import to a static import.
Impact: The import of '* as vite from 'vite'' at the top level of 'index.js' changes the module loading strategy from a peer-resolved dynamic import to a static import. If the consuming project has a different Vite version than the one bundled with SvelteKit, this can lead to duplicate Vite instances, breaking the 'instanceof' check in 'isRunnableDevEnvironment' and potentially causing security-relevant misconfigurations (e…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
1 participant
fixes the vite ecosystem ci error https://github.com/vitejs/vite-ecosystem-ci/actions/runs/32693769091/job/97332021820#step:7:1811
There is no point to us calling
resolve_peer(...)for a Vite instance in thevite/index.jsfile anymore since we don't use to run the client build during a server build (SvelteKit 2 behaviour). Removing this should avoid the vite-ecosystem-ci error where the Vite runnerinstanceofcheck fails from two differently resolved Vite instances (the one that started the build and the one from ourresolve_peer).Source merge-base:
2eed638671196bb4734d7e17f5f6d70a2b704b5dSource head:
82d2454fc48faed4f6c400b49cc79da7ea0520af