Skip to content

Client side scripts guide update#12091

Merged
yanthomasdev merged 18 commits into
mainfrom
client-side-scripts-rework
Aug 8, 2025
Merged

Client side scripts guide update#12091
yanthomasdev merged 18 commits into
mainfrom
client-side-scripts-rework

Conversation

@OliverSpeir
Copy link
Copy Markdown
Contributor

@OliverSpeir OliverSpeir commented Jul 28, 2025

Description (required)

Essentially the goal of this PR is to "modernize" this page, making it more focused and correct.

We trim down some unnecessary examples, bring important information to the top of the page, and cut out some information that is no longer accurate and not really the docs' job to go over.

A little background on the changes:

Astro changed the way scripts work in v5 by "direct rendering" which inlines scripts and this page was written assuming that the scripts would always be externalized. Inline type module scripts do indeed block rendering, and instead of clarifying exactly how all that stuff works we cut most of it out because it's not astro specific behavior.

Related issues & labels (optional)

These changes were discussed here: #11165

Additional notes from Sarah

  • a few pages that link to now-changed section headers have been updated, and affects some translated pages, so we'll need to i18nIgnore those pages (additional additional note from Yan: in this case, a lunaria directive on merge to main! Here's the code: @lunaria-track:src/content/docs/en/**/*.mdx)
  • Whatever we decide to call "opt out of processing/unprocessed scripts/inline scripts" will need to be updated on the View Transitions page (line 525) -- this should be the only link check that fails now

@netlify
Copy link
Copy Markdown

netlify Bot commented Jul 28, 2025

Deploy Preview for astro-docs-2 ready!

Name Link
🔨 Latest commit 52c6dc3
🔍 Latest deploy log https://app.netlify.com/projects/astro-docs-2/deploys/6895f1efc5a59a000812c9e0
😎 Deploy Preview https://deploy-preview-12091--astro-docs-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@astrobot-houston
Copy link
Copy Markdown
Contributor

astrobot-houston commented Jul 28, 2025

Lunaria Status Overview

🌕 This pull request will trigger status changes.

Learn more

By default, every PR changing files present in the Lunaria configuration's files property will be considered and trigger status changes accordingly.

You can change this by adding one of the keywords present in the ignoreKeywords property in your Lunaria configuration file in the PR's title (ignoring all files) or by including a tracker directive in the merged commit's description.

Tracked Files

File Note
ar/basics/astro-components.mdx Localization changed, will be marked as complete.
ar/basics/layouts.mdx Localization changed, will be marked as complete.
en/basics/astro-components.mdx Source changed, localizations will be marked as outdated.
en/guides/client-side-scripts.mdx Source changed, localizations will be marked as outdated.
en/guides/upgrade-to/v5.mdx Source changed, localizations will be marked as outdated.
en/guides/view-transitions.mdx Source changed, localizations will be marked as outdated.
hi/basics/astro-components.mdx Localization changed, will be marked as complete.
it/basics/astro-components.mdx Localization changed, will be marked as complete.
pl/basics/astro-components.mdx Localization changed, will be marked as complete.
pl/basics/layouts.mdx Localization changed, will be marked as complete.
zh-tw/basics/astro-components.mdx Localization changed, will be marked as complete.
Warnings reference
Icon Description
🔄️ The source for this localization has been updated since the creation of this pull request, make sure all changes in the source have been applied.

Comment thread src/content/docs/en/guides/client-side-scripts.mdx Outdated
Copy link
Copy Markdown
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fantastic @OliverSpeir !! Thank you so much for tackling this!

I've left mostly high-level comments and questions here re: the overall content (not close editing). I think what you've got is really great, and just want to find a balance of keeping some content that I still think provides some helpful context given the expected audience of some of these sections.

Comments below!

Comment thread src/content/docs/en/guides/client-side-scripts.mdx
Comment thread src/content/docs/en/guides/client-side-scripts.mdx
Comment thread src/content/docs/en/guides/client-side-scripts.mdx
Comment thread src/content/docs/en/guides/client-side-scripts.mdx Outdated
Comment thread src/content/docs/en/guides/client-side-scripts.mdx Outdated
Comment thread src/content/docs/en/guides/client-side-scripts.mdx Outdated
Comment thread src/content/docs/en/guides/client-side-scripts.mdx Outdated
Comment thread src/content/docs/en/guides/client-side-scripts.mdx Outdated
Comment thread src/content/docs/en/guides/client-side-scripts.mdx Outdated
Comment thread src/content/docs/en/guides/client-side-scripts.mdx Outdated
Comment thread src/content/docs/en/guides/client-side-scripts.mdx Outdated
Comment thread src/content/docs/en/guides/client-side-scripts.mdx Outdated
Copy link
Copy Markdown
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, next round of review comments from me! (For everyone!) 🙌

Comment thread src/content/docs/en/guides/client-side-scripts.mdx Outdated
Comment thread src/content/docs/en/guides/client-side-scripts.mdx Outdated
Comment thread src/content/docs/en/guides/client-side-scripts.mdx
Comment thread src/content/docs/en/guides/client-side-scripts.mdx Outdated
Comment thread src/content/docs/en/guides/client-side-scripts.mdx
Comment thread src/content/docs/en/guides/client-side-scripts.mdx
Comment thread src/content/docs/en/guides/client-side-scripts.mdx Outdated
Comment thread src/content/docs/en/guides/client-side-scripts.mdx Outdated
Comment thread src/content/docs/en/guides/client-side-scripts.mdx Outdated
@sarah11918 sarah11918 added the improve or update documentation Enhance / update existing documentation (e.g. add example, improve description, update for changes) label Aug 1, 2025
@github-actions github-actions Bot added the i18n Anything to do with internationalization & translation efforts - ask @YanThomas for help! label Aug 1, 2025
OliverSpeir and others added 7 commits August 3, 2025 18:39
Co-authored-by: Sebastian Beltran <bjohansebas@gmail.com>
Co-authored-by: Sarah Rainsberger <5098874+sarah11918@users.noreply.github.com>
Co-authored-by: Sarah Rainsberger <5098874+sarah11918@users.noreply.github.com>
Co-authored-by: Sarah Rainsberger <5098874+sarah11918@users.noreply.github.com>
@OliverSpeir
Copy link
Copy Markdown
Contributor Author

I think beside final polishing etc what's left to do is

  1. Wording of the previous "Opt out of processing" heading which currently is "Unprocessed scripts"
    • The goal is to handle both the negative and the positive use case ( where they want to purposefully make it not processed, and where they want to figure out why wasn't their script processed ). The nuance is that now scripts are inlined by default, so you can have an inlined and processed script or if you don't want it processed you can set is:inline to make it inlined and not processed
  2. Updating the files that referred to the old heading "Using <script> in Astro"
  3. Update the files that refer to old "Opt out of processing" heading

Comment thread src/content/docs/en/guides/client-side-scripts.mdx Outdated
@OliverSpeir
Copy link
Copy Markdown
Contributor Author

OliverSpeir commented Aug 4, 2025

For the sake of this PR I don't think we add the section parallel to this style page section, although I do think it should be documented somewhere/sometime, on talking and docing I got the sense that this was "reference" content and not "guide" content.

For posterity the reason I think it should be documented is because Astro happens to use the vite.build. assetsInlineLimit config for deciding when to inline astro scripts and this isn't something that a user would assume this setting to affect as vite doesn't typically control whether a script gets inlined or not based on this.

from vite docs on the config option:

assets that are smaller than this threshold will be inlined as base64 URLs
https://vite.dev/config/build-options.html#build-assetsinlinelimit

@sarah11918
Copy link
Copy Markdown
Member

Also noting here that in a Discord thread we discussed the option of adding back that additional content as a recipe, as we've done for similar content:

We could consider a Bundle Control recipe, which can then tie this action to a specific use case. We have a similar example where we used to show renaming output build file names right in a guide, but this was pretty advanced and we didn't necessarily want to suggest people actually do it unless they had a good reason to. So we removed it. Someone found it still in an old translation and thought it was helpful and could describe when it would be useful, so we added it back as a recipe here: https://docs.astro.build/en/recipes/customizing-output-filenames/

I could see doing something similar here, and then it can address both styles and scripts as necessary, itself could link to Vite, and then both pages could link to it?

Co-authored-by: Sarah Rainsberger <5098874+sarah11918@users.noreply.github.com>
@OliverSpeir OliverSpeir changed the title Client side scripts guide rework Client side scripts guide update Aug 4, 2025
Comment thread src/content/docs/en/guides/client-side-scripts.mdx Outdated
Comment thread src/content/docs/en/guides/view-transitions.mdx Outdated
@yanthomasdev
Copy link
Copy Markdown
Member

Added an additional note to @sarah11918's additional notes regarding i18n. I'll be back with a full review once we are done.

Copy link
Copy Markdown
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final preview looks good to me! Just need a Final Boss @yanthomasdev review, and to make sure to add the Lunaria statement from the PR description when merging! 🎉

Copy link
Copy Markdown
Member

@yanthomasdev yanthomasdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work here! A few suggestions from me 🙌

Comment thread src/content/docs/en/guides/client-side-scripts.mdx Outdated
Comment thread src/content/docs/en/guides/client-side-scripts.mdx Outdated
Comment thread src/content/docs/en/guides/client-side-scripts.mdx Outdated
OliverSpeir and others added 4 commits August 8, 2025 06:36
Co-authored-by: Yan <61414485+yanthomasdev@users.noreply.github.com>
Co-authored-by: Yan <61414485+yanthomasdev@users.noreply.github.com>
Co-authored-by: Yan <61414485+yanthomasdev@users.noreply.github.com>
@yanthomasdev yanthomasdev merged commit 7b7e7c3 into main Aug 8, 2025
10 checks passed
@yanthomasdev yanthomasdev deleted the client-side-scripts-rework branch August 8, 2025 12:57
ArmandPhilippot added a commit to ArmandPhilippot/astro-docs that referenced this pull request Aug 9, 2025
angelmarfil pushed a commit to angelmarfil/docs that referenced this pull request Aug 13, 2025
@lunaria-track:src/content/docs/en/**/*.mdx
--------------------------------------------------------------------------------------------------------



Co-authored-by: Sarah Rainsberger <5098874+sarah11918@users.noreply.github.com>
Co-authored-by: Sebastian Beltran <bjohansebas@gmail.com>
Co-authored-by: Yan <61414485+yanthomasdev@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

i18n Anything to do with internationalization & translation efforts - ask @YanThomas for help! improve or update documentation Enhance / update existing documentation (e.g. add example, improve description, update for changes)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants