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

Can't dynamically import a node module with a variable #14102

Open
7 tasks done
Looskie opened this issue Aug 14, 2023 · 32 comments
Open
7 tasks done

Can't dynamically import a node module with a variable #14102

Looskie opened this issue Aug 14, 2023 · 32 comments
Labels
enhancement New feature or request

Comments

@Looskie
Copy link

Looskie commented Aug 14, 2023

Describe the bug

Hi! I've ran into an issue where I can't import a dependency from node modules with a variable. But without a variable, it works. I also tried using a variable for a "local" import and that also works.

I've attached a minimal reproduction, that uses dayjs as an example.

Reproduction

original: https://stackblitz.com/edit/vitejs-vite-rb9v1k?file=src%2FApp.tsx

vite 5.4.2 (same issue): https://stackblitz.com/edit/vitejs-vite-cc4fvd?file=src%2FApp.tsx

(check console)

Steps to reproduce

No response

System Info

System:
    OS: macOS 13.3
    CPU: (8) arm64 Apple M1
    Memory: 119.72 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.3.1 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 9.8.0 - /usr/local/bin/npm
    pnpm: 8.2.0 - /usr/local/bin/pnpm
  Browsers:
    Chrome: 115.0.5790.170
    Firefox Nightly: 114.0a1
    Safari: 16.4

Used Package Manager

yarn

Logs

TypeError: Failed to resolve module specifier 'dayjs/locale/en'

Validations

@stackblitz
Copy link

stackblitz bot commented Aug 14, 2023

@hai-x
Copy link
Contributor

hai-x commented Aug 15, 2023

It will get some warnings.

The above dynamic import cannot be analyzed by Vite.
See https://github.com/rollup/plugins/tree/master/packages/dynamic-import-vars#limitations for supported dynamic import formats. If this is intended to be left as-is, you can use the /* @vite-ignore */ comment inside the import() call to suppress this warning.

Try @rollup/plugin-dynamic-import-vars plugin and modify the vite.config.ts

import dynamicImportVars from '';
// https://vitejs.dev/config/
export default defineConfig({
  plugins: [
    dynamicImportVars({
      // options
    }),
    react(),
  ],
});

But the plugin has some limitations on importing path. See more: https://github.com/rollup/plugins/tree/master/packages/dynamic-import-vars#limitations

@Looskie
Copy link
Author

Looskie commented Aug 15, 2023

@haijie-x

Hi! Thanks for the quick reply. The limitations of this plugin specifically says that node_module imports wont work, which adds more problems than I originally started with.

@webJose
Copy link

webJose commented Aug 17, 2023

According to the page listed in the warning, you are not complying with Vite's limitations as far as I can see:

  • days/locale/en doesn't start with ./ or ../.
  • ./meow/${test} doesn't have a file extension and doesn't have a file pattern.

@Looskie
Copy link
Author

Looskie commented Aug 17, 2023

According to the page listed in the warning, you are not complying with Vite's limitations as far as I can see:

  • days/locale/en doesn't start with ./ or ../.
  • ./meow/${test} doesn't have a file extension and doesn't have a file pattern.

I am aware. Both of the cases that you provided from the reproduction still worked though, and is a smaller part of the issue at hand.

@bluwy
Copy link
Member

bluwy commented Aug 21, 2023

Making this work will be tricky but not impossible. When you do import(`dayjs/locale/${'en'}`), what happens is that Vite needs to glob dayjs/locale/* to identify the possible values of * and bundle it all up. We're able to support the glob syntax today with optimizeDeps.include, and this translates to supporting the glob syntax in import.meta.glob too as the dynamic import will be transpiled as an import.meta.glob.

@bluwy bluwy added enhancement New feature or request and removed enhancement: pending triage labels Aug 21, 2023
@goldyfruit
Copy link

goldyfruit commented Oct 3, 2023

I'm using vite-plugin-federation to load remote components. I'm having (I think) the same issue (not very familiar with Vite/VueJS).

Basically this works:

defineAsyncComponent(() => import('ovosSkillPersonalOpenvoiceos/MainComponent'));

But this doesn't:

const componentName = 'ovosSkillPersonalOpenvoiceos/MainComponent';
defineAsyncComponent(() => import(componentName));

I tried this as well but without success:

const componentName = 'SkillPersonalOpenvoiceos/MainComponent';
defineAsyncComponent(() => import('ovos' + componentName));

Not sure if there is a solution/workaround.

Thanks 👍

@bluwy
Copy link
Member

bluwy commented Oct 4, 2023

You need strings inside import() to be static like the first example. Most bundlers require that so it's statically analyzable for bundling. I think it's not quite related to this issue.

@goldyfruit
Copy link

You need strings inside import() to be static like the first example. Most bundlers require that so it's statically analyzable for bundling. I think it's not quite related to this issue.

Thanks

@Brandon-Ln
Copy link

Brandon-Ln commented Oct 12, 2023

Making this work will be tricky but not impossible. When you do import(`dayjs/locale/${'en'}`), what happens is that Vite needs to glob dayjs/locale/* to identify the possible values of * and bundle it all up. We're able to support the glob syntax today with optimizeDeps.include, and this translates to supporting the glob syntax in import.meta.glob too as the dynamic import will be transpiled as an import.meta.glob.

@bluwy
Excuse me, I'd like to know if the reason for not supporting this feature for now is because the Vite or Rollup team assesses the implementation cost may be too high? What confuses me is that dynamic import of a node package name literal is currently supported, so I think the bundler should be able to statically analyze the absolute path of the node package correctly. Why does adding support for glob matching functionality bring the restriction of having to import resources with relative paths?

I have a webpack project that can correctly split chunks as intention when both variables and packages appear in the path, but I encountered this feature unsupported during the migration to a Vite project.

const lang = await getLanguage()
const jsonFile = (await import(`@company/i18n-json/${lang}/index.json`)).default

May I ask if this feature is hard to implement due to differences in dynamic import parsing algorithms between webpack and Vite/Rollup under the hood? Or is there any other design consideration or trade off for Vite/Rollup chose not to support it, like hoping follow to the ESM specification strictly etc.?

I understand the complexity of the module search algorithm currently with the introduction of Node submodules, and I appreciate the work of the community team. Please forgive me if I said anything incorrect or offended.

@goldyfruit
Copy link

Making this work will be tricky but not impossible. When you do import(`dayjs/locale/${'en'}`), what happens is that Vite needs to glob dayjs/locale/* to identify the possible values of * and bundle it all up. We're able to support the glob syntax today with optimizeDeps.include, and this translates to supporting the glob syntax in import.meta.glob too as the dynamic import will be transpiled as an import.meta.glob.

@bluwy Excuse me, I'd like to know if the reason for not supporting this feature for now is because the Vite or Rollup team assesses the implementation cost may be too high? What confuses me is that dynamic import of a node module name literal is currently supported, so I think the bundler should be able to statically analyze the absolute path of the node module correctly. Why does adding support for glob matching functionality bring the restriction of having to import resources with relative paths?

I have a webpack project that can correctly split chunks as intention when both variables and node modules appear in the path, but I encountered this feature unsupported during the migration to a Vite project.

const lang = await getLanguage()
const jsonFile = (await import(`@company/i18n-json/${lang}/index.json`)).default

May I ask if this feature is hard to implement due to differences in dynamic import parsing algorithms between webpack and Vite/Rollup under the hood? Or is there any other design consideration or trade off for Vite/Rollup chose not to support it, like hoping follow to the ESM specification strictly etc.?

I understand the complexity of the module search algorithm currently with the introduction of Node submodules, and I appreciate the work of the community team. Please forgive me if I said anything incorrect or offended.

I am totally supporting your comment. When you start with Javascript and frameworks, most of the documentations recommend to use Vite because its the "new" way to start new project but Vite still lack some of the features supported by the "old ways" such as Webpack.

I got really frustrated when I had to quit Vite to start fresh with Webpack.

Its not a complaint but a feedback and I'm grateful for what the Vite community did (thanks 👍 ).

@bluwy
Copy link
Member

bluwy commented Oct 12, 2023

We're not intentionally not supporting it, it's that this usecase wasn't considered in the initial design/support for glob imports. It started out as a way to glob relative files so it worked for relative paths, but then it slowly extended out, e.g. supporting globbing with resolve.alias, and now the request for node_modules.

node_modules in particular is harder because packages can declare specific ways to export files. In particular, figuring out the possible values of lang within @company/i18n-json/${lang}/index.json isn't an easy task. There is the logic implemented today, but making it work for dynamic imports is another task.

We're definitely open to a PR if someone submits one. For accepted enhancements we have the "enhancement" label.


If you'd like to see this feature implemented, you can also leave a 👍 to the issue.

If you'd like to workaround at the meantime, it's doable to also write a plugin that generates the list of dynamic imports and inject it into your code. Essentially doing the globbing manually if you know what the langs are.

@Looskie
Copy link
Author

Looskie commented Oct 12, 2023

We're not intentionally not supporting it, it's that this usecase wasn't considered in the initial design/support for glob imports. It started out as a way to glob relative files so it worked for relative paths, but then it slowly extended out, e.g. supporting globbing with resolve.alias, and now the request for node_modules.

node_modules in particular is harder because packages can declare specific ways to export files. In particular, figuring out the possible values of lang within @company/i18n-json/${lang}/index.json isn't an easy task. There is the logic implemented today, but making it work for dynamic imports is another task.

We're definitely open to a PR if someone submits one. For accepted enhancements we have the "enhancement" label.

If you'd like to see this feature implemented, you can also leave a 👍 to the issue.

If you'd like to workaround at the meantime, it's doable to also write a plugin that generates the list of dynamic imports and inject it into your code. Essentially doing the globbing manually if you know what the langs are.

Would love to help out, time is a bit of an issue though, so whenever I get the time I can try to whip something up. Any pointers to how/what can be done would be greatly appreciated if its not much of an ask @bluwy ❤️

@bluwy
Copy link
Member

bluwy commented Oct 13, 2023

The main starting point would be the importMetaGlob Vite plugin. This line in particular is what Vite has today to resolve glob paths:

const resolved = normalizePath(
(await resolveId(glob, importer, {
custom: { 'vite:import-glob': { isSubImportsPattern } },
})) || glob,
)

(search for resolveId in that file to follow how the path is resolved)

Also note that dynamic imports with variables are also transpiled as import.meta.glob today, so you'll need to make sure the transpiled code is correctly accessing from the result of import.meta.glob:

const { rawPattern, glob } = result
needDynamicImportHelper = true
s.overwrite(
expStart,
expEnd,
`__variableDynamicImportRuntimeHelper(${glob}, \`${rawPattern}\`)`,
)

@Brandon-Ln
Copy link

We're not intentionally not supporting it, it's that this usecase wasn't considered in the initial design/support for glob imports. It started out as a way to glob relative files so it worked for relative paths, but then it slowly extended out, e.g. supporting globbing with resolve.alias, and now the request for node_modules.

node_modules in particular is harder because packages can declare specific ways to export files. In particular, figuring out the possible values of lang within @company/i18n-json/${lang}/index.json isn't an easy task. There is the logic implemented today, but making it work for dynamic imports is another task.

We're definitely open to a PR if someone submits one. For accepted enhancements we have the "enhancement" label.

If you'd like to see this feature implemented, you can also leave a 👍 to the issue.

If you'd like to workaround at the meantime, it's doable to also write a plugin that generates the list of dynamic imports and inject it into your code. Essentially doing the globbing manually if you know what the langs are.

Thk for your detailed explanation! I will keep an eye on it.

For those who may face the same problem as me can try this temporarily as long as your project lists the dependencies correctly:

  const lang = await getLanguage();
  let json = undefined;
  if (compatRelativePath) {
    /**
     * We manually use relative path import to make the rollup plugin happy.
     */
    json = (await import(`./node_modules/@company/i18n-json/${lang}/index.json`)).default;
  } else {
    json = (await import(`@company/i18n-json/${lang}/index.json`)).default;
  }

I know this may feel a bit dirty and definitely not science, but engineering :)

@Looskie
Copy link
Author

Looskie commented Oct 15, 2023

The main starting point would be the importMetaGlob Vite plugin. This line in particular is what Vite has today to resolve glob paths:

const resolved = normalizePath(
(await resolveId(glob, importer, {
custom: { 'vite:import-glob': { isSubImportsPattern } },
})) || glob,
)

(search for resolveId in that file to follow how the path is resolved)

Also note that dynamic imports with variables are also transpiled as import.meta.glob today, so you'll need to make sure the transpiled code is correctly accessing from the result of import.meta.glob:

const { rawPattern, glob } = result
needDynamicImportHelper = true
s.overwrite(
expStart,
expEnd,
`__variableDynamicImportRuntimeHelper(${glob}, \`${rawPattern}\`)`,
)

Okay, spent the entire day trying to figure it out and I thought I got pretty close? But ultimately ended up no where.

Here's my digging for the record:

I started at the files that you reference, I found out that toAbsoluteGlob didn't get called. I went up the ladder to see where it would stop, and resolvedFileName in transformDynamicImport was returning undefined.

if (importSource[1] !== '.' && importSource[1] !== '/') {
const resolvedFileName = await resolve(importSource.slice(1, -1), importer)
if (!resolvedFileName) {
return null
}

Which got me into a huge rabbithole to see what and where resolve was defined.

I landed here

import fs from 'node:fs'

I landed here because other dynamic imports were resolving from

const handler =
'handler' in plugin.resolveId
? plugin.resolveId.handler
: plugin.resolveId
const result = await handleHookPromise(
handler.call(ctx as any, rawId, importer, {
assertions: options?.assertions ?? {},
custom: options?.custom,
isEntry: !!options?.isEntry,
ssr,
scan,
}),
)
if (!result) continue

So following resolve.ts I found out that dynamic node_module imports fall through this function

async resolveId(id, importer, resolveOpts) {

But non dynamic node_module imports (Like import(dayjs/locale/en)) would be returned at

if (bareImportRE.test(id)) {
const external = options.shouldExternalize?.(id, importer)
if (
!external &&
asSrc &&
depsOptimizer &&
!options.scan &&
(res = await tryOptimizedResolve(
depsOptimizer,
id,
importer,
options.preserveSymlinks,
options.packageCache,
))
) {
return res
}

The reason this condition is failing is because of the depsOptimizer being undefined for dynamic node_module imports, which is to be expected.

I wish I could say I know where to go from here but I really don't. I'll try again tomorrow but I can only really commit my weekends to this. any thoughts @bluwy ?

Details that don't really effect the end result (i dont think)

the import analyzer warns you most likely because of this guybedford/es-module-lexer#137 (correct me if im wrong)

Which is used here

;[imports, exports] = parseImports(source)

and is undefined here (because .n is undefined from the package)

let specifier = importSpecifier.n

@bluwy
Copy link
Member

bluwy commented Oct 26, 2023

Sorry for the late reply. I've been focusing on getting Vite 5 out and missed this. resolvedFileName being undefined is expected I believe because IIUC you're trying to resolve some-pkg/**/*.js which doesn't really exist.

You'd first have to extract the library name some-pkg and resolve that, then glob the files within that library. There's a utility that does that:

/**
* Expand the glob syntax in `optimizeDeps.include` to proper import paths
*/
export function expandGlobIds(id: string, config: ResolvedConfig): string[] {

Though we can't really use it completely to fix this cleanly. We might need to carve out a part in the resolve.ts plugin for import globs only that can support resolving the globs (using some code from that utility). The standard resolve spec in Vite doesn't resolve globs by default.

Thanks for taking a look at it. Understandably it's quite a big task to finish up. I'll also remove the "contribution welcome" label for now since it's a bigger task than expected, and we usually reserve it for simpler stuff. But we'll still accept contributions if you like to look deeper into it!

@Looskie
Copy link
Author

Looskie commented Oct 27, 2023

No worries! I haven't had much time over the weekend anyways. I'll take a last look over this weekend to see if I can hack something together with the new info. I really appreciate your guidance ❤️

@wldrve
Copy link

wldrve commented Nov 3, 2023

Hi, how far are you on this matter?

I would also need the feature to "dynamically globally import node_modules" for localization purposes. I am working on an app with 9 languages/locales and have two dependencies, which have to be set correctly, with a proper locale. I do not want to load all the localization settings, for all the languages. This worked with Webpack perfectly and now switching to Vite, this is the only "open" thing, for me to fix. Thanks and kind regards!

@MuratGundes
Copy link

Hello!
Do we have any progress or at least any workaround? We are also facing the same issue since we migrated off CRA to Vite.

pierluigigiancola added a commit to pierluigigiancola/experiment-vite that referenced this issue Dec 10, 2023
NOTE: I didn't manage to make it work the same way webpack worked.
The problem is that vite runs after tsc, so the env variables get replaced after the transpilation.
Thanks to import-meta-env I make it work but I lose the code splitting since the build
now can dynamically change the env variables...
Look for this discussion: vitejs/vite#14102
@renie
Copy link

renie commented Dec 13, 2023

I am not quite sure if this is exact the same issue, but letting the comment anyway because I could not find it explicitly this way.

I was trying to make this work:

    translations = !lang.startsWith('en')
        ? (await import(`./${lang}.js`)).default
        : (await import(`./en-us.js`)).default

It would work on dev, but build would create just translation file to en-us. So "production" version would work just in english.

So after digging a bit on what @bluwy said on

Also note that dynamic imports with variables are also transpiled as import.meta.glob

I've came to this solution below. Worked pretty well for me, loading just the correct file at the correct time, for both dev and build. Let me know if this is an anti-pattern or something like that.

    const modules = import.meta.glob('./*-*.js', { import: 'default' })
    translations = !lang.startsWith('en')
        ? await modules[`./${lang}.js`]()
        : await modules[`./en-us.js`]()

@karlhorky
Copy link
Contributor

karlhorky commented Feb 10, 2024

I recently filed a similar bug (maybe the same):

I'm trying to use the Vite define option + dynamic import to pass in import paths during build time, and the imported files are not available in the final build:

Repo: https://github.com/karlhorky/vite-dynamic-import-build-missing-files

main.ts

await import(`./x/${__NAME__}.ts`);

vite.config.ts

import { defineConfig } from 'vite';

export default defineConfig(({ mode }) => {
  return {
    build: { target: 'esnext' },
    define: {
      __NAME__: JSON.stringify('dynamic'),
    },
  };
});

This works in development but breaks after build:

Uncaught TypeError: Failed to fetch dynamically imported module: http://localhost:8000/assets/x/dynamic.ts

Is this is the same issue described above?

@ParsaArvanehPA
Copy link

You need strings inside import() to be static like the first example. Most bundlers require that so it's statically analyzable for bundling. I think it's not quite related to this issue.

Fixed my problem, thx.

@TheElegantCoding
Copy link

@Brandon-Ln maybe not related but he type of the import is any i cant get the json types of the default values of a dynamic import, this is not possible ?

@Brandon-Ln
Copy link

@Brandon-Ln maybe not related but he type of the import is any i cant get the json types of the default values of a dynamic import, this is not possible ?

In my understanding, dynamic import in this use case is a runtime behavior, so it may not be able to be inferred by static analysis. In my use case, I mark the returned result as an unknown and then perform subsequent type deduction through runtime type check(so-called type predicates)

@TheElegantCoding
Copy link

TheElegantCoding commented Jun 7, 2024

but how @Brandon-Ln ? i am not getting it, in your example, you import a dynamic json, i could also have "index.json", "about-us.json" with different structures.

const lang = await getLanguage()
const jsonFile = (await import(`@company/i18n-json/${lang}/${json}.json`)).default as unknown 

jsonFile is unknown and i dont get type completion from an unknown file also i don't have a type predicate to use since i have more than one json file

@Brandon-Ln
Copy link

Brandon-Ln commented Jun 7, 2024

but how @Brandon-Ln ? i am not getting it, in your example, you import a dynamic json, i could also have "index.json", "about-us.json" with different structures.

const lang = await getLanguage()
const jsonFile = (await import(`@company/i18n-json/${lang}/${json}.json`)).default as unknown 

jsonFile is unknown and i dont get type completion from an unknown file also i don't have a type predicate to use since i have more than one json file

What I mean is like:

type JsonFile = Record<string, string> // You can replace the types you are interested in here

function jsonFileValidator(target: unknown): target is JsonFile {
  // Your custom runtime check logic, It is just an example here
  return typeof target === "object" && !!target
}

const lang = await getLanguage()
const jsonFile: unknown = (await import(`@company/i18n-json/${lang}/${json}.json`)).default

if (!jsonFileValidator(jsonFile)) {
  // Error handling
  throw new Error("Unsupported json files.")
}
/**
 * 'JsonFile' is infererd
 */
jsonFile

But if you require to infer all possible fields of the JSON file that may be obtained completely, I think it may be impossible in this scenario👀

klimeryk added a commit to klimeryk/recalendar.js that referenced this issue Jun 21, 2024
Use dynamic imports, even though Vite does not quite yet support this
for node_modules. See vitejs/vite#14102

Tried different approaches, including `import.meta.glob` to get all the
locale names first. But the static analysis still can't figure out all
the possible imports, so I get the dreaded warning.
klimeryk added a commit to klimeryk/recalendar.js that referenced this issue Jul 5, 2024
CRA is no longer actively developed: https://github.com/facebook/create-react-app
The dependencies are old and causing numerous security warnings,
conflicts with other packages, etc.
Let's switch to Vite - looks promising, more lightweight and modern.
Without the bells and whistles of Next.JS that is not needed for my
needs here.

Rough steps needed to make this work:
- Fix i18n
- Fixes needed to make the worker work with vite
 - Make the Worker be type: module.
 - Ensure no dnd, etc. dependencies get loaded - for that, I had to
   split out the constants and utils from components to separate files.
- Switch to ~ for root src path
 - Like it more than @
 - @ feels like root for DNS
 - ~ feels more natural, being home path on *nix
- Use dynamic imports, even though Vite does not quite yet support this for node_modules. See vitejs/vite#14102
klimeryk added a commit to klimeryk/recalendar.js that referenced this issue Jul 5, 2024
CRA is no longer actively developed: https://github.com/facebook/create-react-app
The dependencies are old and causing numerous security warnings,
conflicts with other packages, etc.
Let's switch to Vite - looks promising, more lightweight and modern.
Without the bells and whistles of Next.JS that is not needed for my
needs here.

Rough steps needed to make this work:
- Fix i18n
- Fixes needed to make the worker work with vite
 - Make the Worker be type: module.
 - Ensure no dnd, etc. dependencies get loaded - for that, I had to
   split out the constants and utils from components to separate files.
- Switch to ~ for root src path
 - Like it more than @
 - @ feels like root for DNS
 - ~ feels more natural, being home path on *nix
- Use dynamic imports, even though Vite does not quite yet support this for node_modules. See vitejs/vite#14102
@minimit
Copy link

minimit commented Aug 28, 2024

Seems fixed in vite 5.4.2

@karlhorky
Copy link
Contributor

karlhorky commented Aug 28, 2024

Seems fixed in vite 5.4.2

@minimit Ohh nice, can you show your demo code that is working?

Do you know which change in the changelog entries for 5.4.2 fixed this?

@Looskie
Copy link
Author

Looskie commented Aug 28, 2024

Seems fixed in vite 5.4.2

Also interested in your demo code. I just bumped my repro that was provided above and it doesn't seem fixed.

updated repro: https://stackblitz.com/edit/vitejs-vite-cc4fvd?file=src%2FApp.tsx

@minimit
Copy link

minimit commented Aug 30, 2024

Seems fixed in vite 5.4.2

@minimit Ohh nice, can you show your demo code that is working?

Well now i've a node package that does a dynamic import inside it's own package and it works, before it gave error, it's this line:

import(`./modules/${name.toLowerCase()}${suffix}.mjs`)

But from your demo, if you try to do a dynamic import inside node modules it still gives error.

@wldrve
Copy link

wldrve commented Sep 16, 2024

Still not working for my case, which means, I can not dynamically import node_modules dependencies when needed. I tried with Vite dynamic import and with rollup dynamic imports and no luck.

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

No branches or pull requests