feat: auto-exclude svelte dependencies in vite.optimizeDeps #145
Conversation
dominikg
changed the title
| return !!pkg.svelte; | ||
| } | ||
|
|
||
| const COMMON_DEPENDENCIES_WITHOUT_SVELTE_FIELD = [ |
There was a problem hiding this comment.
should we include everything that comes with a new SvelteKit project?
"devDependencies": {
"@sveltejs/kit": "next",
"@types/cookie": "^0.4.0",
"@typescript-eslint/eslint-plugin": "^4.19.0",
"@typescript-eslint/parser": "^4.19.0",
"eslint": "^7.22.0",
"eslint-config-prettier": "^8.1.0",
"eslint-plugin-svelte3": "^3.2.0",
"prettier": "~2.2.1",
"prettier-plugin-svelte": "^2.2.0",
"svelte": "^3.34.0",
"svelte-check": "^2.0.0",
"svelte-preprocess": "^4.0.0",
"tslib": "^2.0.0",
"typescript": "^4.0.0"
},
"type": "module",
"dependencies": {
"@fontsource/fira-mono": "^4.2.2",
"@lukeed/uuid": "^2.0.0",
"cookie": "^0.4.1"
}
There was a problem hiding this comment.
most of it was covered already by the prefix checks, added the remaining ones. thanks!!
| if (path.length === 3) { | ||
| log.warn.once(`encountered deep svelte dependency tree: ${path.join('>')}`); | ||
| } |
There was a problem hiding this comment.
Should we be warning when we encounter this scenario? The crawler should be able to continue down the tree, just that it's weird for this to ever happen. I think we can remove these lines, or maybe convert to debug if it helps for debugging.
There was a problem hiding this comment.
Such a deeply nested svelte component package is a sign something is afoot with the project and a warning is in order imho.
I agree that it is weird and tbh i'm hoping this never occurs in the wild. Still i'd like to know if it does and debug output would not be noticed in regular use.
To avoid spam, the warning is logged only once per run and not again if the tree is even deeper. foo>bar>baz gets logged, but foo>bar>baz>qoox won't.
There was a problem hiding this comment.
I suppose it's good to know if this ever happens in the wild, but I'm concern if it would annoy the end-user since it may be something not fixable on their end. We might get an issue/pr to remove that line too. Since if it's a warning, the end-user should be able to suppress or fix it (which sometimes isn't possible).
Or we could leave it there and increment the number if someone do report it, so we know there exist an atrocity like that 😆
There was a problem hiding this comment.
made it debug for now. I guess such a deep path would cause other issues and if it cannot be resolved with the package authors we can add back a warning (and an extra option to filter some logs?)
|
manually tested it with npm@6 and yarn1 + yarn3 with pnp. and it works 🥳 |
…to ensure find doesn't stop early
A simple test has been added.