nuxt / module-builder

Complete solution to build and ship Nuxt modules.
MIT License
211 stars 22 forks source link

importing from `runtime/` dir doesn't add extension #261

Closed harlan-zw closed 1 month ago

harlan-zw commented 2 months ago

With https://github.com/nuxt/module-builder/commit/7a68e1e we no longer allow the build of the module to use any runtime code.

This makes it impossible (?) to re-use code between the module and the runtime without extracting it into a separate package, as the runtime can already not import anything relative outside of the runtime folder.

In most cases this is desirable but there are edge cases where sharing small snippets of code is needed. For example in the OG Image module normalising font input data is required at both levels due to dynamic runtime support.

Previous code would let me import from import { normaliseFontInput } from './runtime/pure.ts' (https://github.com/nuxt-modules/og-image/blob/v3.0.0-rc.52/src/module.ts#L52) and the function would be bundled as part of the main module code. See https://unpkg.com/browse/nuxt-og-image@3.0.0-rc.52/dist/module.mjs (normaliseFontInput)

I don't think reverting the change is needed but it would be good to discuss what the approach here should be without writing duplicate code or publishing extra packages.

danielroe commented 2 months ago

It's still possible, you just should manually add the file you're importing to your externals list, for example:

https://github.com/nuxt-modules/tailwindcss/pull/840/files#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519R83

What it won't do is bundle and include it as a separate file - which I think is a better default behaviour (particularly on the types front).

harlan-zw commented 2 months ago

I see, thank you for the reference. It is good that it's not bundling duplicate code now.

I did notice this behaviour but I had some memory of the bundled imports needing to have extensions, so this would require imports to have an .mjs extension right?

For example, a bundled module would have code like the below, which might break when consumed?

// module.mjs
import { util } from './runtime/shared'

Let's close this if the missing extension isn't an issue, I couldn't find the exact doc on this.

danielroe commented 2 months ago

I believe the extension will be added when compiling...

(If not, let me know and we can reopen.)

harlan-zw commented 2 months ago

This does not appear to be the case unless I'm missing something obvious, see: https://stackblitz.com/edit/stackblitz-starters-hhhjue?file=my-module%2Fsrc%2Fmodule.ts

cd my-module
pnpm i && pnpm dev:prepare && pnpm prepack

You can even see the playground is throwing an error trying to load it because of the missing extension.

danielroe commented 2 months ago

You're quite right. Ideally we fix this in the build step 🙏

Probably needs to be handled upstream in unbuild.

antfu commented 1 month ago

Encounter the same in Nuxt DevTools, here is my temporary patch:

https://github.com/nuxt/devtools/blob/51b1990966bc917c37554f9c6d3105c768a6161a/packages/devtools/build.config.ts#L26-L53

hooks: {
    // Patch @nuxt/module-builder@0.6.0 not adding .mjs extension for runtime files
    // https://github.com/nuxt/module-builder/issues/261
    'rollup:options': (_, options) => {
      options.plugins ||= []
      if (!Array.isArray(options.plugins))
        options.plugins = [options.plugins]

      const runtimeDir = fileURLToPath(new URL('./src/runtime', import.meta.url))
      options.plugins.unshift({
        name: 'unbuild:runtime-build:patch',
        async resolveId(id, importter) {
          if (!id.includes('runtime'))
            return
          const resolved = await this.resolve(id, importter, { skipSelf: true })
          if (resolved?.id.startsWith(runtimeDir)) {
            let id = resolved.id
            if (!id.endsWith('.mjs'))
              id += '.mjs'
            return {
              external: true,
              id,
            }
          }
        },
      })
    },
  },