search

harlan-zw / nuxt-site-config

Unifying site config with powerful and flexible APIs, for module authors and users.
https://nuxtseo.com/site-config
61 stars 3 forks source link

Runtime should not be depending on module entry #3

Closed pi0 closed 6 months ago

pi0 commented 10 months ago

Hi. Escalating this from https://github.com/unjs/nitro/issues/1629 while I was debugging, noticed your module is trying to bundle c12 into the runtime.

[12:21:18 AM]  ERROR  "rm" is not exported by "node_modules/.pnpm/unenv@1.7.3/node_modules/unenv/runtime/node/fs/promises/index.mjs", imported by "node_modules/.pnpm/c12@1.4.2/node_modules/c12/dist/index.mjs".

This happens because the utils in runtime directory are trying to import from nuxt-site-config-kit which is the module entry containing build time dependencies including @nuxt/kit.

You should never combine them. If you really need shared dependencies, they should be exported from a subpath export of the module, ideally residing within src/runtime directory.

BTW nuxt should have failed with import protection plugin not sure why is not caught. /cc @danielroe

harlan-zw commented 10 months ago

Hmm interesting, I figured that the tree shaker would have been able to remove the dependency since there's no side effects :thinking: Will update soon.

harlan-zw commented 10 months ago

Actually, digging further into it, I can't see any evidence of what you're describing @pi0, could you provide the code that's causing the issue?

There's nothing in the runtime directory importing from nuxt-site-config-kit. The only thing that uses it is the module entry itself.

harlan-zw commented 10 months ago

Okay on even further digging, it seems like the issue is that the site-config-stack package has a dependency for @nuxt/kit.

  "dependencies": {
    "@nuxt/kit": "^3.6.5",
    "defu": "^6.1.2",
    "pkg-types": "^1.0.3",
    "ufo": "^1.2.0"
  }

However, this dependency is never used in the code, this was just a mistake when copy+pasting the package.json over.

So not sure why it's getting bundled exactly. Seems like something that could open up bugs and extra weight in a number of repos, so ideally upstream can handle it better. I don't think it's anything that the import protection code can help with.

danielroe commented 10 months ago

Adding nuxt/kit in a package.json shouldn't result in it getting bundled. I'd suggest we should keep looking.

harlan-zw commented 6 months ago

I'm going to close this as I believe it was a false positive anyway (see my above comment).