search

lilnasy / gratelets

A collection of purpose-built, small, third-party integrations for Astro.
The Unlicense
105 stars 6 forks source link

dynamic imports: prevent repeated scripts and styles #88

Closed stevenwoodson closed 7 months ago

stevenwoodson commented 7 months ago

Hi @lilnasy!

Background

I've been working through a proof of concept Astro implementation that pulls content from a CMS (in my case Sanity) and went down the rabbit hole of noticing that all components styling and JS dependencies get loaded on every page regardless of whether they're used or not.

I came across this roadmap discussion that mentioned your dynamic-import integration and was very excited to give it a try.

From my initial testing, it does indeed isolate dependencies on a per-page basis Though as I dug in a bit further with more complex pages, I noticed that if a component was used on the page more than once then the style and JavaScript dependencies are added to the page that same number of times. It doesn't affect the page rendering but does add a considerable amount to the pageload/

This Proof of Concept

Rather than just opening an issue, I wanted to dive in further and understand what's going on first, and then offer a suggestion. This is a bit of a crude example, and I'm sure it can be refined, but it gets the job done.

This addition keeps track of the components that have been processed in processed, if it was already then the styles, links, and scripts are suppressed so they don't get added again.

In my tests, the page still renders fine and I don't see duplicate styles or JS in using the standard dev or build commands.

changeset-bot[bot] commented 7 months ago

🦋 Changeset detected

Latest commit: 7118bf66bf77a117acfff6e961304af15ffe9ef8

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

lilnasy commented 7 months ago

Yes, you are exactly right!

I had considered de-duplicating against the normally-added scripts and styles, but not against the ones added by other dynamic imports.

lilnasy commented 7 months ago

Thanks for taking this on! I can write the tests and WeakMap tracking later this week if you don't beat me to it.

stevenwoodson commented 7 months ago

That'd be fantastic, I'll admit I've not had cause to use WeakMap before so I certainly defer to the experts on that part.

I'm glad the overall logic is sound, thanks for offering to take it that next step further! Please do @ me if there's any more I can do otherwise.

stevenwoodson commented 7 months ago

Seems my edit worked a bit too well, after some further testing I realized that the CSS/JS additions only happened once ever so the first page that used that component got the CSS and JS while the rest didn't.

I just committed a change that also keeps track of the current pathname and resets the processed array when a new path is encountered. I'm unsure if this is the ideal way to do this, but it is working in my additional tests.

stevenwoodson commented 7 months ago

Hey @lilnasy, Just pulled the latest edits and can confirm this is still working as expected.

Thanks for touching it up and adding tests, I agree this looks ready to merge! :shipit:

lilnasy commented 7 months ago

@stevenwoodson Awesome, incredible work!