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

[🐞] import.meta.glob either works in dev and doesn't in prod, or works in prod but gets slower and slower as the number of imported files increases in dev #5498

Open
maiieul opened this issue Nov 29, 2023 · 3 comments
Labels
STATUS-1: needs triage New issue which needs to be triaged TYPE: bug Something isn't working

Comments

@maiieul
Copy link
Contributor

maiieul commented Nov 29, 2023

Which component is affected?

Qwik Rollup / Vite plugin

Describe the bug

When using import.meta.glob as is, which means with eager: false as it is the default, Qwik isn't able to understand Vite's new bundling for the server build.

So when I run pnpm preview, I get:

❌ Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/home/maieul/dev/qwik-meta-glob-repro/server/assets/@qwik-city-not-found-paths.js' imported from /home/maieul/dev/qwik-meta-glob-repro/server/assets/entry.preview-27aca4ec.mjs


If now I use eager: true, Qwik is able to create a proper /server output in preview/prod, but then the dev server gets slower and slower as the number of imported files increases.


I thought the problem was simply that when using import.meta.glob, Vite places the dynamic import.meta.glob modules in /server/assets, and so that it could be fixed by putting back all the modules in /server.

So if you comment out

    build: {
      rollupOptions: {
        output: process.env.npm_lifecycle_event === "build.preview" ? {
          chunkFileNames:
          '[name]-[hash].mjs',
        }: undefined,
      },
    },

in the repro's vite.config.ts, you will see that it works just fine in preview. But on larger codebases, it creates other errors.

Ex from qwik-ui's docs site using the same build.rollupOptions.output config:
Screenshot from 2023-11-29 09-59-49

Workaround

Thanks to @wmertens, we found that using import { isDev } from '@builder.io/qwik/build'; can workaround this issue.

The idea is to conditionally use eager:false in development, so that it's not slow, and eager: true in preview/prod so that qwik can take care of creating the lazy-load boundaries as it normally does.

Ex:

import {
  type Component,
  component$,
  useSignal,
  useTask$,
} from "@builder.io/qwik";
import { isDev } from "@builder.io/qwik/build";

const metaGlobComponents: any = import.meta.glob("/src/components/*", {
  import: "default",
  eager: isDev ? false : true,
});

export default component$<{ name: string }>(({ name }) => {
  const MetaGlobComponent = useSignal<Component<any>>();
  const componentPath = `/src/components/${name}.tsx`;

  useTask$(async () => {
    MetaGlobComponent.value = isDev
      ? await metaGlobComponents[componentPath]()
      : metaGlobComponents[componentPath];
  });

  return <>{MetaGlobComponent.value && <MetaGlobComponent.value />}</>;
});

You can replace <Glob name="example1" /> with <GlobWorkaround name="example1" /> in /routes/index.tsx to see the workaround working.

Potential ways to fix this

  1. We can potentially fix this in the QwikCity or QwikVite plugins by making sure that the plugins can understand the Vite import.meta.glob output structure. Overall the bugs I'm describing seem to be related to modules not being properly imported, so if Qwik can figure out a way to handle those Vite-created dynamic imports, then this is probably the right fix.
  2. If (1) doesn't work, we can also create an AST transform in the optimizer like it's done for props_destructuring.rs to automatically transform import.meta.glob into the workaround version above. But that sounds more like a patch than a real fix.

Reproduction

https://github.com/maiieul/qwik-meta-glob-repro

Steps to reproduce

To see the bug:

  • pnpm install
  • pnpm preview

To see the workaround:

  • replace <Glob name="example1" /> with <GlobWorkaround name="example1" /> in /routes/index.tsx.

System Info

System:
    OS: Linux 6.5 Fedora Linux 38 (Workstation Edition)
    CPU: (16) x64 13th Gen Intel(R) Core(TM) i5-1340P
    Memory: 19.78 GB / 31.04 GB
    Container: Yes
    Shell: 3.6.1 - /usr/bin/fish
  Binaries:
    Node: 20.9.0 - /usr/local/bin/node
    npm: 10.1.0 - /usr/local/bin/npm
    pnpm: 8.10.5 - ~/.local/share/pnpm/pnpm
  Browsers:
    Chrome: 119.0.6045.159
  npmPackages:
    @builder.io/qwik: ^1.2.19 => 1.2.19 
    @builder.io/qwik-city: ^1.2.19 => 1.2.19 
    undici: ^5.26.0 => 5.28.1 
    vite: ^4.4.11 => 4.5.0

Additional Information

This is not the most urgent issue as there is a workaround, but I think this is still an important one to fix as using import.meta. glob can save a lot of development time. It allows using a pattern-matching string to define all the files to import instead of having to manually write bucket files where we have to specify which files to import one by one.

I also tried using dynamic imports, but it didn't work and probably has even more drawbacks than import.meta.glob (e.g. imports such as icons/material/zoom_in.txt?raw cannot work)

I think it would be great to have a guide on lazy-loading in the docs with the workaround documented in it until we have a fix for this. I'll be working on it.

@maiieul maiieul added STATUS-1: needs triage New issue which needs to be triaged TYPE: bug Something isn't working labels Nov 29, 2023
@mhevery
Copy link
Contributor

mhevery commented Dec 10, 2023

import.meta.glob is vite feature not Qwik. Why not use regular imports?

@maiieul
Copy link
Contributor Author

maiieul commented Dec 10, 2023

This is not the most urgent issue as there is a workaround, but I think this is still an important one to fix as using import.meta. glob can save a lot of development time. It allows using a pattern-matching string to define all the files to import instead of having to manually write bucket files where we have to specify which files to import one by one.

TLDR: import.meta.glob can save a lot of dev time/effort.

@maiieul
Copy link
Contributor Author

maiieul commented Dec 11, 2023

import.meta.glob is currently usable in Qwik with the workaround, but it is definitely not great DX. I know it's a Vite feature but other developers will want to use it too as it saves dev time/effort. That's why I think it would be great if QwikVite plugin knew how to work with it OOB.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
STATUS-1: needs triage New issue which needs to be triaged TYPE: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants