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

Rendering content collection entries through a loop with imported astro components causes broken scripts and styles #6672

Closed
1 task
putnikproj opened this issue Mar 27, 2023 · 7 comments · Fixed by #7062
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority) feat: content collections Related to the Content Collections feature (scope)

Comments

@putnikproj
Copy link

What version of astro are you using?

2.1.7

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

What browser are you using?

Chrome

Describe the Bug

The new Content Collection API lets you import mdx files and render their content anywhere using the .render() method, and only imports those components and their styles and scripts when they're used on a page. This works perfectly when generating routes from content - on each page an entry, its scripts and styles are executed and bundled. But when I try to render multiple entries on one page through a loop, scripts and styles don't work.

// pages/page.astro
---
const entries = await getCollection('someColletion');
---
{entries.map(async entry => {
	// Scripts and styles don't work in <Content />
	const { Content } = await entry.render();
	return <Content mdProps="work" />; 
})}
// content/someCollection/some-mdx.mdx
import AstroComponent from 'pathToComponent/AstroComponent.astro';
<AstroComponent />
// components/AstroComponent.astro
<!-- Some html -->
<script>/* Some scripts */</script>
<style>/* Some styles */</style>

Everything works when explicitly tell astro which components to render (see reproduction):

---
const { Content } = await entries[0].render();
const { Content: Content2 } = await entries[1].render();
---
<Content />
<Content2 />

I want to render my mdx components with imported nested astro components through an array, as this is the only way to use astro components and prevent bundling of unused scripts and styles (#6328)

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-b1xdf7-ptottr?file=src%2Fcomponents%2FRenderEntry.astro,src%2Fpages%2Findex.astro,src%2Fpages%2Ffinal.astro

Participation

  • I am willing to submit a pull request for this issue.
@matthewp matthewp added the - P4: important Violate documented behavior or significantly impacts performance (priority) label Mar 27, 2023
@JerryWu1234
Copy link
Contributor

I would like to check it.

@JerryWu1234 JerryWu1234 mentioned this issue May 11, 2023
@matthewp
Copy link
Contributor

This is because script/style propagation is dependent on the template being static. We examine the template and see that there are components which contains styles and then wait on that portion before sending the <head> tag out. Making the template dynamic in this way prevents this.

I anticipated this problem when working on head propagation but couldn't think of a good solution. I think some explicit API is probably needed here. For example if there was a {Astro.propagate(entries.map(...))} or perhaps a <Propagate>{entries.map(...)}</Propagate>

@JerryWu1234
Copy link
Contributor

JerryWu1234 commented May 12, 2023

This is because script/style propagation is dependent on the template being static. We examine the template and see that there are components which contains styles and then wait on that portion before sending the <head> tag out. Making the template dynamic in this way prevents this.

I anticipated this problem when working on head propagation but couldn't think of a good solution. I think some explicit API is probably needed here. For example if there was a {Astro.propagate(entries.map(...))} or perhaps a <Propagate>{entries.map(...)}</Propagate>

const { Content } = await entries[0].render();
const { Content: Content2 } = await entries[1].render();
---
<Content />
<Content2 />

It sounds a little weird.
It doesn't need to add anything in the upon example.
But we must add extra code in another example.
However, I'll continue to finish my PR. It probably is not a good idea though , but there is still a different perception.
hope can have a little help

@putnikproj
Copy link
Author

I wouldn't say the issue is solved. It simply adds wrong html markup with script and style tags. Yes, the browser displays the expected output, but the final generated html is completely wrong and unacceptable (Reproduction: https://stackblitz.com/edit/github-b1xdf7-mcrjxh. npm run build, dist/final.html).
Screenshot 2023-05-31 at 17 16 44
Moreover, during testing, I noticed another bug with the content collections rendering function, which is related to the first one (the same reproduction, but dist/index.html). Although all scripts and styles are correctly propagated to the top <head> tag, additional redundant nested html document structure is created inside the body, at the place where I use the <Content /> component.
Screenshot 2023-05-31 at 17 27 15

@JerryWu1234
Copy link
Contributor

JerryWu1234 commented Jun 1, 2023

I tried a lot, but didn't find the best way to solve it So I think it can only be used as a temporary solution.
I saw that issue in the image you provided. because of this code
the reason is the same function key can't be identified in the map .

@bholmesdev
Copy link
Contributor

Reopening since this introduced a separate bug, and causes unexpectedly broken Markup as noted by @putnik-projects

@bholmesdev bholmesdev reopened this Jun 6, 2023
@natemoo-re natemoo-re added the feat: content collections Related to the Content Collections feature (scope) label Jul 18, 2023
@lilnasy
Copy link
Contributor

lilnasy commented Aug 31, 2023

This issue has been reported several times, and is currently being tracked in #7761.

@lilnasy lilnasy closed this as not planned Won't fix, can't repro, duplicate, stale Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority) feat: content collections Related to the Content Collections feature (scope)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants