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

fix(build): preserve getOptions in CJS wrapper #421

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

TonyFresneau
Copy link

@TonyFresneau TonyFresneau commented Nov 5, 2024

Hello again @danielroe,

I started my day with a warning from @nuxtjs/sitemap about @nuxtjs/i18n options not being available.

image

  1. The warning originates from @nuxtjs/sitemap trying to access the options of @nuxtjs/i18n using nuxtModule.getOptions
  2. While the ESM version of the module has the getOptions method (added by defineNuxtModule), it's lost in the CJS wrapper
  3. This happens because the current CJS stub generation in @nuxt/module-builder only preserves meta and getMeta, but not getOptions (i guess)

More details :

I investigated the warning and found its source in @nuxtjs/sitemap at https://github.com/nuxt-modules/sitemap/blob/main/src/module.ts#L219, which leads to https://github.com/nuxt-modules/sitemap/blob/f52cafd726e2c5a7ebf8f768eda5d0f9befac9cc/src/util/kit.ts#L37.

The issue occurs when @nuxtjs/sitemap tries to use @nuxtjs/i18n options: nuxtModule.getOptions is undefined (where nuxtModule is @nuxtjs/i18n), resulting in no return value from nuxtModule.getOptions(inlineOptions, nuxt).

The root cause is in the module loading process at https://github.com/nuxt/nuxt/blob/7177e81442559fee574e9460b4d76a780bb98211/packages/kit/src/module/install.ts#L85, where getOptions is not being preserved in the CJS wrapper.

Looking at module.cjs from @nuxtjs/i18n, getOptions is indeed missing. I tested by manually adding it to my node_modules and it all work, no warning anymore from @nuxtjs/sitemap :

module.exports = function(...args) {
  return import('./module.mjs').then(m => m.default.call(this, ...args))
}
const _meta = module.exports.meta = require('./module.json')
module.exports.getMeta = () => Promise.resolve(_meta)
// Added getOptions preservation
module.exports.getOptions = function(...args) {
  return import('./module.mjs').then(m => m.default.getOptions.call(this, ...args))
}

I discovered that this part was implemented using this package, so here I am, proposing an update to build.ts.

I only started looking into this today, so there's a chance I might be mistaken. The issue might not be here and could be related to @nuxtjs/i18n or @nuxtjs/sitemap.

@harlan-zw , this might interest you. I'm using version 6.1.4 of @nuxtjs/sitemap

Previously, the CJS wrapper only preserved 'meta' and 'getMeta' properties from Nuxt modules, causing issues with modules like sitemap which rely on the getOptions method. This commit adds getOptions to the CJS wrapper, ensuring full module API compatibility between ESM and CJS environments.
@danielroe
Copy link
Member

i think this is probably fixed in nuxt/nuxt#29799 but that doesn't mean we shouldn't also improve the stub

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants