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

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

Closed
harlan-zw opened this issue Apr 29, 2024 · 6 comments · Fixed by #275
Closed

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

harlan-zw opened this issue Apr 29, 2024 · 6 comments · Fixed by #275
Labels
bug Something isn't working upstream

Comments

@harlan-zw
Copy link
Collaborator

harlan-zw commented Apr 29, 2024

With 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/[email protected]/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.

Copy link
Member

danielroe commented Apr 29, 2024

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
Copy link
Collaborator Author

harlan-zw commented Apr 29, 2024

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.

@harlan-zw harlan-zw changed the title No longer possible to have build-time code share runtime code Best practices on sharing build-time and runtime code Apr 29, 2024
@danielroe
Copy link
Member

danielroe commented Apr 29, 2024

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

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

@danielroe danielroe closed this as not planned Won't fix, can't repro, duplicate, stale Apr 29, 2024
@harlan-zw
Copy link
Collaborator Author

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.

@harlan-zw harlan-zw reopened this Apr 29, 2024
@harlan-zw harlan-zw changed the title Best practices on sharing build-time and runtime code Shared build-time and runtime code requires extension Apr 29, 2024
Copy link
Member

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

Probably needs to be handled upstream in unbuild.

@danielroe danielroe added the bug Something isn't working label Apr 29, 2024 — with Volta.net
@danielroe danielroe changed the title Shared build-time and runtime code requires extension importing from runtime/ dir doesn't add extension Apr 29, 2024
@antfu
Copy link
Member

antfu commented May 10, 2024

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/[email protected] 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,
            }
          }
        },
      })
    },
  },

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working upstream
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants