Skip to content

Scripts and Event Handling: elaborate on module scripts#4974

Merged
lilnasy merged 8 commits into
withastro:mainfrom
lilnasy:module-scripts
Oct 10, 2023
Merged

Scripts and Event Handling: elaborate on module scripts#4974
lilnasy merged 8 commits into
withastro:mainfrom
lilnasy:module-scripts

Conversation

@lilnasy
Copy link
Copy Markdown
Contributor

@lilnasy lilnasy commented Oct 5, 2023

Description

Clarify a few common points of confusion regarding processing of scripts.

Related issues & labels

None

Direct link to preview

https://deploy-preview-4974--astro-docs-2.netlify.app/en/guides/client-side-scripts/#script-processing

@netlify
Copy link
Copy Markdown

netlify Bot commented Oct 5, 2023

Deploy Preview for astro-docs-2 ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit ee1a1f6
🔍 Latest deploy log https://app.netlify.com/sites/astro-docs-2/deploys/6524484ffab186000807ef1d
😎 Deploy Preview https://deploy-preview-4974--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 site configuration.

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.

Some really nice additions here, @lilnasy ! See my comments below and let's see if we can really get this just right!

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
Comment thread src/content/docs/en/guides/client-side-scripts.mdx Outdated
```

To avoid bundling the script, you can use the `is:inline` directive.
#### All processed scripts become module scripts
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
#### All processed scripts become module scripts
#### module scripts by default
All scripts processed and bundled by Astro become module scripts by default.

This feels more like a "key fact" than a "section heading". Can we have a more generic heading that is like a "thing", and then lead with the key fact?

Alternatively, this might mean that this really isn't its own section at all? (Or, the section is "advantages of ..."? Consider whether this really is A. Section. I kind of think maybe it's not?

Also, since this sentence then becomes kind of short, we can through "bundled" in here again because we do use that word a lot, and the reinforcement of terms people use/are familiar with helps.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What do you think about "Why type=module" as the heading? It is introduced as a bullet point a couple of lines above.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't love the "why" part but I like type=module better than anything so far.

Not everything needs to be a heading, and I'm trying to think of justification for this. Headings are useful for navigation, for separating content sections, and as visual breaks. Given that this is a list, we already have some natural visual break for the eyes.

I'm still not convinced this "section" is anything more than how Astro processes your scripts. This still seems content-wise to be in that category.

And, it's not awesome practice for content/navigation to use questions as headers. When people are scanning, they want to know what this section is about. I think the most appropriate heading, if in fact we do need one (which I still am not convinced we do!) is "type-module benefits" -- that is what this section is about.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It does not have to be a section. I'd be happy with just a sentence that introduces the list.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated

Comment thread src/content/docs/en/guides/client-side-scripts.mdx Outdated
lilnasy and others added 2 commits October 10, 2023 00:04
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
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.

Another fine contribution! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants