Dynamically importing components in Astro will load all components located in the components folder causing console errors

moritzlaube commented 2 years ago

What version of `astro` are you using?

1.2.5

Are you using an SSR adapter? If so, which one?

None

What package manager are you using?

npm

What operating system are you using?

Mac

Describe the Bug

I am using a CMS (Strapi) to deliver my content. Strapi (as well as Prismic and others) have the concept of Dynamic Zones. It let's you freely choose and combine components. Within the JSX section of my Astro page, I am looping through the available components coming from Strapi and attaching the respective Component to the HTML like so:

  {product.attributes.dynamicZone.map(z => {
    switch (z.__component) {
      case 'marketing.benefits':
        return <Benefits data={z} style="margin-top: var(--space-xl-2xl)" />
      case 'marketing.text-only':
        return <Responsibility data={z} />
      case 'marketing.blog-feed':
        return <BlogFeed data={z} style="margin-top: var(--space-2xl-3xl)" />
      case 'product.product-ingredients':
        return <Ingredients data={z} style="margin-top: var(--space-xl-2xl)" />
      case 'marketing.contact':
        return <Contact client:visible data={z} client:visible style="margin-top: var(--space-xl-2xl)" />
    }
  })}

Initially, I was importing all possible components in between the fences, even though I would only use some of them in the CMS. Just to be flexible. But I experienced some problems with undefined variables with components that had a script tag within them but weren't used by the CMS.

So I tried dynamic imports which is supported by Vite. So I figured, It must work with Astro as well. I was dynamically importing the components within the fences based off the data coming from Strapi:

const dynamicZoneComponents = product.attributes.dynamicZone.map(z => {
  switch (z.__component) {
    case 'marketing.benefits':
      return 'Benefits'
    case 'marketing.text-only':
      return 'TextOnly'
    case 'marketing.blog-feed':
      return 'BlogFeed'
    case 'product.product-ingredients':
      return 'Ingredients'
    case 'marketing.contact':
      return 'Contact'
  }
})

const loadComponents = componentsArray => {
  return Promise.all(componentsArray.map(async entry => (await import(`../../components/${entry}.astro`)).default))
}

const Components = await loadComponents(dynamicZoneComponents)

It works, and Components is returning an Array of valid Astro Component Factories. But here it comes: No matter, what I am doing – when importing the components with the above described dynamic import, Astro is always loading all components I have in my project, causing all sorts of errors and warnings in my console.

On Stackblitz, forked a simple Astro-starter and added a very basic Test.astro component into the components folder – not using it anywhere else.

On the index.astro page, instead of simply importing the Card component, I created a dynamicZoneComponents array that normally would come from the CMS containing a simple string of the component, which I then dynamically import.

Please open the DevTools and within the network tab search for Test. You'll find the component has been loaded even though it hasn't been used anywhere else. This causes a problem as soon as you referenced DOM-elements within those components that aren't present on the current page.

I am wondering if this is a known issue or if you have some other workaround to dynamically import components.

Cheers, Moritz

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-dnitn6?file=src/pages/index.astro

Participation

[ ] I am willing to submit a pull request for this issue.

DaniloSandner commented 2 years ago

i am facing the exact same issue - any help would be appreciated. how to get around this problem?

btw: my use case is based on wordpress (with ACF - flexible content fields) as a headless cms data source. (i would have loved to use strapi for it too but, as far as i know, strapi doesn't support multiple dynamicZones nested, does it?)

moritzlaube commented 2 years ago

(i would have loved to use strapi for it too but, as far as i know, strapi doesn't support multiple dynamicZones nested, does it?)

Hi @DaniloSandner! Concerning Strapi: I think, you can have one Dynamic Zone within a component. But you can use that component anywhere else. Also in another component. So technically, you should be able to nest Dynamic Zones. Just give it a try.

matthewp commented 2 years ago

This is because scripts are hoisted into the head. We don't know at runtime if you are going to use Test or not.

There are a couple of things you can do to avoid this:

Use <script is:inline> instead. This won't hoist the script and it will be added at runtime, so only added if you actually use it on the page.
Move the subset of components you want to dynamically import into another subfolder. Like src/components/marketing and import from that. I believe doing this won't import the other components in src/components. You still have the hoisting problem, but only for components that are part of this subfolder.

Since there's nothing to change here, going to close, but happy to continue discussing.

DaniloSandner commented 2 years ago

thanks a lot for you feedback @matthewp !

i believe the intention here is to conditionally choose which components to import at buildtime (not runtime) - at least in my case. maybe i'm missing out on something... but if i explicitly say to import one single file from a given folder and then i get ALL files from this folder imported, it makes no sense to me.

still looking / hoping for a clean way to solve this issue - is there a way to conditionally and dynamically import single files / components at buildtime?

matthewp commented 2 years ago

No, because entry is not known until runtime. So Vite has to include everything matching your expression as a dependency of this module so that it's available just in case its needed.

If you use static import statements for all of the possible modules you might choose from instead of the dynamic import() expression, that's another option.

DaniloSandner commented 2 years ago

i believe all the code is in the frontmatter and so entry is indeed known at buildtime, isn't it? also in the example from @moritzlaube : https://stackblitz.com/edit/github-dnitn6?file=src/pages/index.astro - the code is in the frontmatter (frontmatter = buildtime - right? please correct me, if i'm wrong - just trying to learn)

moritzlaube commented 2 years ago

Thanks, @matthewp for your fast reply as always!

I have to agree to @DaniloSandner, that dynamic imports really don’t make sense if all components are imported no matter what.

So, there is really no way to conditionally load only the needed components? Since we’re fetching data from the CMS in the frontmatter, we do know which components we’ll need upfront at build time. Can’t we use that knowledge somehow?

Importing all possible components feels convoluted and leads to the prescribed issues - unless going with your is:inline approach. But then you’re missing out on code bundling and minification, I guess.

I’d imagine it’s not a rare use case when working with headless CMSs.

Cheers

torchsmith commented 1 year ago

+1

Skrubbadubba commented 1 year ago

Hi! Know this is an old thread but I have some things to add.

Question: does this always happen? And is there any way to check if it is? I have some very small-scale relational data. I have some markdown files referencing other markdown files. My strategy is to dynamically load the referenced markdown files, and pass them to whatever component is using them. It would maybe make sense to dynamically import these in the child component using the data, however I am using svelte components which do not support top-level await AFAIK :(

Since my project isn't very large-scale, importing all relevant data and just filtering probably isn't going to lead to performance issues or anything.

There other thing is I might have a solution, which is vites import.meta.glob(). It produces an object with file paths as keys and functions which import the modules as values.

I'm thinking OP could use the same approach as described first, but instead of dynamically importing with template literals, accesing the relevant values from import.meta.glob

torchsmith commented 1 year ago

@Skrubbadubba

There other thing is I might have a solution, which is vites import.meta.glob(). It produces an object with file paths as keys and functions which import the modules as values.

I've tried using import.meta.glob myself and had the same issue.

It was a while ago though, could've been fixed by now.

betabong commented 6 months ago

How can this issue be closed? This actually renders Astro practically useless for any CMS projects, or at least it makes the promise of Astro to produce slim and fast code null.

How can we consider this issue resolved? Wouldn't that undermine its core promise of delivering lean and fast code, at least for every CMS project?

page.astro:

---
import A from './components/A.astro'
import B from './components/B.astro'
---
<A/>

It's crucial that the output (page.html) doesn't contain any unused components, like B in this case. While I understand that including B during development by Vite might be acceptable, the final built code must be free of such inclusions.

moritzlaube commented 6 months ago

@matthewp, I have to say, I strongly agree with @betabong concerning the use of Astro with common CMS's. If you want to give clients flexibility in composing their pages, you usually build several Astro components that are reflected in the CMS. The clients then can either use them all, or a few of them, and use them in any order they want. If statically built, it would make total sense for the build to only contain the code of the actually used components. I hope that makes sense.

MrChrisRodriguez commented 6 months ago

I was typing the same thing and @moritzlaube beat me to it.

I can understand Astro loading all of the modules into memory, but when it comes to SSG or SSR, why are those being included in my bundles?

I'm admittedly way out of my depth, but wouldn't you be able to load the entire dependency tree into memory and treeshake whatever isn't used?

betabong commented 6 months ago

Reading further into the issue, it seems to originate from Astro's fundamental technical principle of being able to stream the HTML response during Server-Side Rendering (SSR) (see this troubleshoot paragraph). The idea is to send the head of a page while sub-components might still be "rendering" (or waiting for data), allowing the browser to load and preload resources during that delay. This is a valid point, but it comes at a high cost, in my opinion.

The point becomes invalid for Static Site Generation (SSG). However, let's ignore that for the moment. The most common delay in rendering components is loading (remote) data, which usually happens at the page level. In such cases, you wouldn't see much benefit from streaming, as the head might also be blocked because you want to set head data (title, description, etc.) based on the loaded data.

If a component is blocking the page rendering, it's arguably better practice to load the data on the client-side and display a loading skeleton in the meantime, rather than streaming it.

Furthermore, I question whether streaming works in all deployment environments, especially in Edge functions. If it doesn't, it's an even higher price to pay for a smaller use case.

Ultimately, we all aim to create an optimized website, and including unused resources like CSS and JavaScript is not optimized. For content-rich websites built with component-based pages relying on editorial data (a very common use case), Astro's principles don't seem to fit well.

Optionally turning off streaming entirely could be a step towards a solution. Personally, I question the overall benefit of streaming when it comes at the cost of unoptimized output and added complexity (I can only imagine). This trade-off might be acceptable if streaming provides significant benefits, but I'm skeptical about its practical advantages, especially for content-rich websites built with component-based pages relying on editorial data. In such scenarios, the value proposition of streaming seems diminished, and the associated complexity may outweigh the potential benefits.

moritzlaube commented 6 months ago

Dear @matthewp, is there a chance to open that issue up again or move it to a feature discussion? What's the best way to take those comments into account, which, in my opinion, really touch a crucial point when working with headless CMS's.

moritzlaube commented 6 months ago

To everyone participating in that discussion: Does anyone of you have an idea on how to shine a light back on that issue? Especially, since a few of us spotted the same problem? Opening up a new issue?

betabong commented 6 months ago

I guess opening a new issue would be the way to go. I don't think we're being heard here. We'll have to provide a clear, understandable, reproducible example.

The issue is hardly fixable in a streaming architecture, but I for one would happily exchange streaming for slim (or rather non-bloated) code. A possible fix could be: an option to disable streaming.

tobitestra commented 6 months ago

We are interested in a solution for this as well. We are building an ecommerce website that is 100% fully dynamic, even the route setup is only available in a database.

So we only have two single page routes in the filesystem. One "500.ts" to serve a custom 500 error page if something weird happens. And one "[...index].ts" catchall route, that takes the requested URL, runs to a backend in order to validate the URL against the DB and receive an array of components to display, if its a valid page request.

We currently have the following test components in total in our setup, that pages can use dynamically:

A
B
C
Error
Error404
Error503
Footer
Header
NewslettersignupForm
RandomSlider
Test

Each component carries two script blocks for testing the issue discussed in this topic:

<script is:inline>console.log('A is:inline')</script>
<script>console.log('A')</script>

This is how we display the dynamic components in the "...index.ts" markup section:

...html
{ components && (
        <div>
          {console.log("total components: " + components.length)}
          {components.map(async (component, index) => {
            console.log("component +++ index:", index,'+++ id:', component.id)
            console.log(`awaiting import for: ../components/astro/modules/${component.name}/${component.name}.astro`)
            const DynamicComponent = (await import(`../components/astro/modules/${component.name}/${component.name}.astro`)).default
            return <DynamicComponent key={index} id={component.id} data={component.data} />;
          })}
        </div>
  )}
...html

For our understanding, Astro should only import the components that are needed for this particular request.

When we open a page that carries the following components (defined via the backend api call), in order:

Header
Test
RandomSlider
B
Footer

We see the following SSR Log output:

total components: 5
component +++ index: 0 +++ id: main-header-abc123
awaiting import for: ../components/astro/modules/Header/Header.astro
component +++ index: 1 +++ id: Test-1
awaiting import for: ../components/astro/modules/Test/Test.astro
component +++ index: 2 +++ id: random-slider-guid-qwertz
awaiting import for: ../components/astro/modules/RandomSlider/RandomSlider.astro
component +++ index: 3 +++ id: B-1
awaiting import for: ../components/astro/modules/B/B.astro
component +++ index: 4 +++ id: main-footer-xyz456
awaiting import for: ../components/astro/modules/Footer/Footer.astro

But the browserconsole log tells us a different truth:

Header is:inline
Test is:inline
RandomSlider is:inline
B is:inline
Footer is:inline
A
B
C
Error
Error404
Error503
Footer 
Header
NewsletterSignupForm
RandomSlider
Test

This shows that the inline scripts fire only for those components that were needed in the exact order of their appearance in the components array (first five lines), BUT the regular script blocks from all the unneeded components are popping up too (last eleven lines).

Any idea on how to address and fix this behaviour/issue?

If we move on now, for instance our homepage, including five components, is always going to carry all the js of the whole application (of all other components), even though it just needs the code of five :(

At this moment this is a big showstopper for Astro for us :(

Any help is highly appreciated, thank you a lot!

ematipico commented 6 months ago

Hey all,

Please refrain from commenting on this issue.

This is not an issue, and this isn't a way to properly code split your application with a full dynamic environment. They way bundlers work nowadays is that they can code split your code base if the argument of you pass to the dynamic import is static:

const DynamicComponent = () => import("./components/Card.astro")

Modern bundlers are able to statically analyze your code and understand that "./components/Card.astro" is a code point where the JavaScript and belonging to that component needs to be loaded lazily.

However, this doesn't work if you decide to build dynamically the argument that you pass to import. This isn't Astro, it's how bundlers work nowadays (webpack, rollup, parcel, etc.):

const DynamicComponent = () => import("./components/"+ someVariableComingFromCms +".astro")

The bundler doesn't know what someVariableComingFromCms is, and it's not able to resolve it. I want you all to understand that a bundler is just an analyzer with steroids, but it's not a runtime, and even though it is, you end up shipping that code to production, and if that variable comes from a CMS... well, it's absolutely impossible to know which components need to be pulled.

Some resources:

hv-pul commented 6 months ago

@ematipico I will gladly refrain if there is a working solution for the problem :)

I think however there is a misunderstanding here. Even if I know the possible component names in advance, I'm not sure how to render them dynamically in Astro. Perhaps it is just a syntax thing, but how would you solve this?

I created a Stackblitz for this: https://stackblitz.com/edit/github-keykfb?file=src%2Fpages%2Findex.astro,src%2Fpages%2F[slug].astro,src%2Fcomponents%2FFoo.astro,src%2Fcomponents%2FBar.astro

The dynamic part works like this:

const module =
  slug === "foo"
    ? await import("../components/Foo.astro")
    : await import("../components/Bar.astro");
const Component = module.default;

If you check this out & create a static build, you will see that the scripts & styles from both Foo and Bar components are included in the html for both /foo and /bar. So the issue is NOT that we need to build the import argument dynamically, but that even for static import paths, Astro will import all possible cases.

For Vue 3, we can use defineAsyncComponent, for React I think it would be lazy. What is the equivalent for Astro? As you can see from the feedback in this issue, it is an essential feature.

withastro / astro