Skip to content
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

Break in version 292 in handling local modules #4261

Closed
mike-hogan opened this issue Mar 7, 2022 · 11 comments
Closed

Break in version 292 in handling local modules #4261

mike-hogan opened this issue Mar 7, 2022 · 11 comments
Labels
bug Something isn't working p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc.
Milestone

Comments

@mike-hogan
Copy link

Describe the bug

Using version 291, I can build a small sveltekit app that depends on a local module that depends on lodash.

Using version 292, I get this error:

Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'lodash' imported from /repos/personal/sveltekit-292-break/sveltekit-app/.svelte-kit/output/server/entries/pages/index.svelte.js

Reproduction

https://github.com/mike-hogan/sveltekit-292-break/tree/main

Logs

~/repos/personal/sveltekit-292-break/sveltekit-app % npm run build      

> sveltekit-app@0.0.1 build
> svelte-kit build

vite v2.8.6 building for production...
✓ 16 modules transformed.
.svelte-kit/output/client/_app/manifest.json                    1.15 KiB
.svelte-kit/output/client/_app/layout.svelte-2c9045fd.js        0.53 KiB / gzip: 0.35 KiB
.svelte-kit/output/client/_app/error.svelte-71f51260.js         1.56 KiB / gzip: 0.75 KiB
.svelte-kit/output/client/_app/pages/index.svelte-ccf6b463.js   0.90 KiB / gzip: 0.51 KiB
.svelte-kit/output/client/_app/start-1f006a4c.js                21.15 KiB / gzip: 7.85 KiB
.svelte-kit/output/client/_app/chunks/vendor-1f241f7c.js        78.62 KiB / gzip: 28.71 KiB
vite v2.8.6 building SSR bundle for production...
transforming (1) .svelte-kit/build/index.js"head" is imported from external module "lodash" but never used in "../module/index.js".
✓ 12 modules transformed.
.svelte-kit/output/server/manifest.json                    0.88 KiB
.svelte-kit/output/server/index.js                         64.38 KiB
.svelte-kit/output/server/entries/pages/layout.svelte.js   0.24 KiB
.svelte-kit/output/server/entries/pages/error.svelte.js    0.72 KiB
.svelte-kit/output/server/entries/pages/index.svelte.js    0.38 KiB
.svelte-kit/output/server/chunks/index-2dc61825.js         2.29 KiB
Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'lodash' imported from /Users/mikehogan/repos/personal/sveltekit-292-break/sveltekit-app/.svelte-kit/output/server/entries/pages/index.svelte.js
    at new NodeError (node:internal/errors:371:5)
    at packageResolve (node:internal/modules/esm/resolve:884:9)
    at moduleResolve (node:internal/modules/esm/resolve:929:18)
    at defaultResolve (node:internal/modules/esm/resolve:1044:11)
    at ESMLoader.resolve (node:internal/modules/esm/loader:422:30)
    at ESMLoader.getModuleJob (node:internal/modules/esm/loader:222:40)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:76:40)
    at link (node:internal/modules/esm/module_job:75:36)
> 500 /
    at file:///Users/mikehogan/repos/personal/sveltekit-292-break/sveltekit-app/node_modules/@sveltejs/kit/dist/chunks/index2.js:968:11
    at save (file:///Users/mikehogan/repos/personal/sveltekit-292-break/sveltekit-app/node_modules/@sveltejs/kit/dist/chunks/index2.js:1187:4)
    at visit (file:///Users/mikehogan/repos/personal/sveltekit-292-break/sveltekit-app/node_modules/@sveltejs/kit/dist/chunks/index2.js:1078:3)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)

System Info

Node v16.13.0
Mac OS X 12.2.1 (21D62)

Severity

blocking an upgrade

Additional Information

No response

@benmccann
Copy link
Member

Potentially something changed in Vite. I would say that the way you have setup this project is a bit odd. I'm not sure you can do:

  "dependencies": {
    "module": "file:../module"
  },

I would recommend that a better solution would be to setup a monorepo with pnpm and a pnpm-workspace.yaml file. Both Vite and SvelteKit use pnpm to build our own libraries

@mike-hogan
Copy link
Author

mike-hogan commented Mar 7, 2022

I would recommend that a better solution would be to setup a monorepo with pnpm and a pnpm-workspace.yaml file. Both Vite and SvelteKit use pnpm to build our own libraries

Thanks Ben. My actual setup is a monorepo (rushjs, which uses pnpm). I just went as vanilla as I could to reproduce the problem.

Is your suggestion that I take this to the Vite folks?

@bluwy
Copy link
Member

bluwy commented Mar 7, 2022

Note: In the repro, the module subdirectory needs to run npm i also.

I'm not sure it's an issue in Vite. The error comes from the adapter stage where it's trying to bundle from /repos/personal/sveltekit-292-break/sveltekit-app/.svelte-kit/output/server/entries/pages/index.svelte.js, which contains an import to lodash.

In v291 and v292 I can see both outputs has the same contents, and both are using the same version of Vite, so something must've changed in SvelteKit perhaps, and from the error it looks like SvelteKit is using Node to load that file.

Unfortunately I haven't been following along the development recently to pinpoint the cause of it.

@benmccann
Copy link
Member

If this started happening in 292 then the most likely culprit would be #4192. I'm not sure what in their would cause it though

Do you think you can provide a version of the reproduction that uses pnpm? The current setup is a bit unusual, but we'd like to support pnpm

@mike-hogan
Copy link
Author

Do you think you can provide a version of the reproduction that uses pnpm? The current setup is a bit unusual, but we'd like to support pnpm

Yes of course @benmccann , see if this helps: https://github.com/mike-hogan/sveltekit-292-break-pnpm

Thanks.

@dimfeld
Copy link
Contributor

dimfeld commented Mar 23, 2022

I've been seeing something similar just now and have some idea of what's happening, though not a complete picture yet. #4192 would be responsible for making this error show up during the build, but the underlying problem is still there, and would still fail when you try to actually run the server.

Short version: Vite is bundling files from the monorepo dependencies directly into the server bundle, which then try to include their own dependencies that aren't accessible from the main app package.

I'll use the files in @mike-hogan's latest example repo to explain.

The module dependency tree looks like this:

graph TD
  sveltekit-app --> module --> lodash
Loading

With pnpm (and RushJS, which uses it by default), each package in the monorepo gets its own node_modules directory which contains only its direct dependencies. So the node_modules of sveltekit-app contains module, but not lodash.

When doing the SSR build, lodash is treated as an external, but module is not. I guess this is because it's part of the monorepo and this is detected? Not quite sure how that works. But the result is the files generated in .svelte-kit/output/server integrate the files from module, which import lodash.

Because lodash is not a direct dependency of sveltekit-app, importing from that package doesn't work when using pnpm. But the resulting server output tries to do just that.

// module/index.js
import {head} from 'lodash'
export const x = 1;
import { c as create_ssr_component, e as escape } from "../../chunks/index-b3d44d52.js";
// module/index.js is directly integrated into the output here.
import "lodash";
const x = 1;
const Routes = create_ssr_component(($$result, $$props, $$bindings, slots) => {
  return `<h1>Welcome to SvelteKit</h1>
<p>Visit <a href="${"https://kit.svelte.dev"}">kit.svelte.dev</a> to read the documentation</p>
x = ${escape(x)}`;
});
export { Routes as default };

Workarounds

ssr.external

I haven't fully validated this yet, but it seems that one workaround here is to put the in-monorepo dependencies into ssr.external.

const config = {
    kit: {
          adapter: adapter(),
          vite: () => ({
            ssr: {
              external: ['module']
            }
          })
        }
};

Which changes the output to conform to pnpm's dependency model:

import { c as create_ssr_component, e as escape } from "../../chunks/index-b3d44d52.js";
import { x } from "module";
const Routes = create_ssr_component(($$result, $$props, $$bindings, slots) => {
  return `<h1>Welcome to SvelteKit</h1>
<p>Visit <a href="${"https://kit.svelte.dev"}">kit.svelte.dev</a> to read the documentation
</p>
x = ${escape(x)}`;
});
export { Routes as default };

This isn't ideal but may be the best workaround for now.

It's worth noting that you can't use module as the name here since Node does import { x } from 'module' and that apparently resolves to node:module instead. But other names work and I've pushed some example changes to a fork at https://github.com/dimfeld/sveltekit-292-break-pnpm. The fork has the needed external line in svelte.config.js commented out, but you can uncomment it to see the build pass.

dependency hoisting

I think installing with the --shamefully-hoist option might fix this, but not sure in a monorepo. Anyway that's not actually an option when using RushJS.

Just add the deps

Adding lodash to the main package would work here, but this gets tedious in larger projects.

@benmccann benmccann added this to the 1.0 milestone Mar 23, 2022
@benmccann benmccann added bug Something isn't working p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc. labels Mar 23, 2022
@bhvngt
Copy link

bhvngt commented Apr 2, 2022

Thanks @dimfeld for your investigation. I was facing similar issue with my rushjs + pnpm project. The issue happens with single module node projects using pnpm or npm as package manager.

Your external: [<module>] is working out well for me.

@bhvngt
Copy link

bhvngt commented Apr 21, 2022

This issue has resurfaced in my repo. In spite of ssr: {external: [<module>]} setting, svelte-kit build is not able to resolve the transient dependencies from an internal local project.

The current workaround is to add transient dependencies to the main app project. But like @dimfeld has mentioned, this will be quite unwieldy for the large project.

I have created a sample repo to help anyone reproduce this issue. - https://github.com/bhvngt/sveltekit_rushjs.

Any help here will be very much appreciated.

@rsweeneydev
Copy link

rsweeneydev commented Jul 1, 2022

We were facing the same issue and were able to come up with a simple solution that gives you the benefit of HMR for local modules in your monorepo when running sveltekit dev, and correct handling at build time.

In svelte.config.js add

const config = {
    vite: {
        optimizeDeps: {
            include: ['@company/lib1', '@company/lib2'],
        },
    },
}

By default, vite will treat your local modules as source files in dev mode. Since source files are not bundled in dev mode, the library files are not moved and can resolve the modules they depend on in their node_modules.

At build time, by default vite will also treat your local modules as source files and bundle them with your sveltekit application code. This causes a problem because the bundled code is located in a different location and so the code can't find the packages in the node_modules folder of the sveltekit app.

To solve this we tell vite to optimize our local modules at build time. This treats them as external modules rather than source code. This only affects treatment at build time.

Hope this helps!

@ifiokjr
Copy link

ifiokjr commented Jul 19, 2022

@rsweeney21 does your fix still work for vite@3 it's not working for me since updating.

@Rich-Harris Rich-Harris modified the milestones: 1.0, whenever Jul 20, 2022
@dummdidumm dummdidumm self-assigned this Jan 10, 2023
@dummdidumm
Copy link
Member

Not reproducible anymore given the OP's pnpm reproduction.

@dummdidumm dummdidumm closed this as not planned Won't fix, can't repro, duplicate, stale Jan 10, 2023
@dummdidumm dummdidumm removed their assignment Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc.
Projects
None yet
Development

No branches or pull requests

9 participants