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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 BUG: Astro.fetchContent causes circular dependency when used with fetched content #1247

Closed
josh-collinsworth opened this issue Aug 27, 2021 · 7 comments
Assignees

Comments

@josh-collinsworth
Copy link

What package manager are you using?

npm

What operating system are you using?

Mac

Describe the Bug

I started with Astro's built-in blog starter for a small blog. From there, I wanted to add a Sidebar component that uses Astro.fetchContent to pull in a list of all blog posts.

However, it seems that when viewing one of the blog posts (let's say, post.md), fetching all posts with Astro.fetchContent creates a circular dependency error in the console.

I've created a repo with the minimum conditions to reproduce here. Here's the specific error message:

Circular dependency: /_astro/src/pages/posts/introducing-astro.md.js -> /_astro/src/layouts/BlogPost.astro.js -> /_astro/src/components/Sidebar.astro.js -> /_astro/src/pages/posts/introducing-astro.md.js

Ideally, it would be nice to be able to load an MD file while also viewing that MD file; it's very conceivable that a user would perhaps want a sidebar with a listing of all posts on a post view page.

Steps to Reproduce

Clone this repo and run npm i followed by npm run dev.

The only changes from the default blog starter template are:

  • An added Sidebar.astro component
  • Use of that component in the BlogPost layout

Link to Minimal Reproducible Example (Optional)

https://github.com/josh-collinsworth/astro-circular-dependency-demo

@jasikpark
Copy link
Contributor

I think the problem is that fetchContent returns post.html which is the rendered html of that page, but you're currently editing that page.

Maybe this is a good case for @natemoo-re's idea of allowing user interfaces to augment the shape of data returned, so maybe you could opt out of the html being passed to you?

interface NoHTML {/** ... */};
const posts = Astro.fetch<NoHTML>("**/*.md");

@matthewp matthewp added this to Needs Triage in 馃悰 Bug Tracker via automation Aug 27, 2021
@matthewp
Copy link
Contributor

Deleted a previous comment I know realize was wrong. The underlying issue here is that Astro.fetchContent uses import.meta.globEager internally. What this essentially does is turn that into a bunch of import statements. So what's happen is that the module is importing itself 馃檭

That means that this is actually a bug in snowpack, which I guess should be filtering out imports of itself. I have a feeling that the new compiler will be required to see this one fixed.

@matthewp matthewp moved this from Needs Triage to Blocked in 馃悰 Bug Tracker Aug 27, 2021
@FredKSchott FredKSchott moved this from Blocked to Prioritized in 馃悰 Bug Tracker Nov 19, 2021
@FredKSchott FredKSchott removed the 0.21 label Nov 19, 2021
@jonathantneal
Copy link
Contributor

@matthewp, is this now resolved in the new compiler?

@happycollision
Copy link
Contributor

happycollision commented Jan 20, 2022

Seems not. I just ran into this. (v0.22.15)

Especially devious... it works in development, but not when you build.

@tony-sull
Copy link
Contributor

I've seen similar issues pop up a few times and it always seems to come down to Astro.fetchContent being used in a component to load things like related posts, but the post's markdown file includes a layout which itself wants to pull in more related posts

Three possible solutions (maybe there's more?):

  • This comes down to best practices. If we have a good build error message and call it out in the docs, it's up to the developer to avoid circular dependencies
  • Only use a markdown page's layout when rendering the page, ignore it when loading the page via Astro.fetchContent
  • Only make Astro.fetchContent available in src/pages - probably a bit heavy handed, and may not completely prevent a circular dependency between multiple pages

@happycollision
Copy link
Contributor

happycollision commented Feb 8, 2022

  • This comes down to best practices. If we have a good build error message and call it out in the docs, it's up to the developer to avoid circular dependencies

This is avoidable, for sure, in many cases. But sometimes the only reason you'd need to use JS is to avoid this problem. In my case, I've got a nav menu that is totally dynamically built based off of files in the src/pages. No need to curate it elsewhere. The only reason I need to use JS to build it (along with other hacks) is that the current page must reference itself.

  • Only use a markdown page's layout when rendering the page, ignore it when loading the page via Astro.fetchContent

I like this idea quite a lot: it would work well in the case I laid out above.

@tony-sull
Copy link
Contributor

I'm going to close this one as "Won't fix", but not because it isn't a use case worth solving! This really is a limitation of how Astro.fetchContent() is expected to work, and also a bit related to how markdown is processed whenever the build loads up a *.md file

I just opened an RFC discussion to start the conversation to come up with the right solution for this. The use case of needing to know what pages will be built for navigation menus, sidebars, etc. is really common - please feel free to jump into the RFC discussion to share any ideas, concerns, or examples you'd like to see supported here!

馃悰 Bug Tracker automation moved this from Prioritized to Done Feb 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

No branches or pull requests

7 participants