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

馃挕 RFC: Ability to transpile/bundle <script> tags inside of Astro components #370

Closed
FredKSchott opened this issue Jun 9, 2021 · 17 comments 路 Fixed by #1178
Closed

Comments

@FredKSchott
Copy link
Member

Brought up by @georges-gomes:

I don't know if it makes sense for you, but I would have liked a transpilation and bundle <script> tags in .astro files. Maybe <script type="babel"> should transpile and bundle (babel standalone has something like that). A little like what you have done for css files basically. So I can add these pockets of client side JS but get a proper transpilation and bundled output. (With module/nomodule for cherry on the cake).

We're staying away from Babel at the moment, but I agree that something like this would make sense.

Related: If a script tag exists in a component that appears multiple times on a page, what happens? Does it get deduped? Should it? We could answer this question at the same time.

@matthewp
Copy link
Contributor

Are we talking module script tags?

@georges-gomes
Copy link
Contributor

I would take it for with module and non-module scripts. Module scripts being deferred means they should be treated separately. I don't know if this transpilation/bundle should be done my default on all script tags or only if a "flag" is set on the script tag.

If a script tag exists in a component that appears multiple times on a page

Yes dedup would be nicer, also if the same script tag is used across multiple pages it should generate a chunk. But if a bundler is involved in the process they are not bad at this.

@matthewp
Copy link
Contributor

matthewp commented Jun 10, 2021

What is there to bundle about non-module scripts? I assume you don't mean something like:

<script>
  function someInline() { // code here }
</script>
<h1>Hello world</h1>
<script>
  var another = '....'
</script>

You don't mean combining scripts, right? The problem there is that ordering might very well matter. Especially things like embeds often rely on scripts being placed after other elements.

If you don't mean inline scripts then some clarity on what would be bundled would be helpful for me.


As far as controlling the number of times a script is inserted, it scares me a little if Astro takes on too much knowledge about HTML structure and makes any sort of assumptions. What would be better, I think, is to give the developer the power to make the decision.

For example we have the Astro "global" available. What if it contained component metadata that you could use? Just an idea:

---
const shouldInsertScript = Astro.component.instanceCount === 1
---

<div>...</div>
{shouldInsertScript ? (
  <script>
     /* my script that should be inserted once */
  </script>
) : null}

@FredKSchott FredKSchott added this to Feature Request in 馃悰 Bug Tracker Jun 13, 2021
@FredKSchott
Copy link
Member Author

I think the current state is a real foot-gun for components. for example, if you have a component with a script tag that gets rendered in some list (ex: 25 times on the page) that's 25 repeating scripts on the page. It'll be mostly invisible to the user when this happens. So at best it's just a page weight / render speed concern (and a violation of easy-to-do-the-right-thing and fast-by-default). At worst, things break.

I also agree that this isn't something we should go really deep on. For ex, I'd be -1 on creating a lot of custom config/primitives to control how the script loads, how many times, where on the page, etc.

What about聽having Astro collect all scripts used to render the page, by default, and then moving them to a single place on the final rendered page? We already do this with CSS / style tags for the same reasons, so this feels aligned there.

Obviously we should have some fallback to opt-out and manually control this yourself (on the off chance you want ~25 scripts on the page :). Something like <script :static>? Could apply to <style :static> as well...

@FredKSchott
Copy link
Member Author

This actually could be solved by another idea that we'd been talking about:

  1. We keep existing behavior for <script> tags.
  2. We enable this collecting/moving/reducing logic for all <script type="module">, <script defer>, and <script async> since they are already decoupled from any ordering concerns in the browser (and therefore, I can't think of a single use-case where you'd want repeating on the page, since there's no guarantee on run order).

@georges-gomes
Copy link
Contributor

Makes sense to me. And it's perfect first step anyway.

@matthewp
Copy link
Contributor

One thing I think we're glossing over a little here is, what is the exact use case for having a script in a component in the first place? I can See a couple:

  • In an app you might create a <CommonHead /> component that contains all of the stuff you put in your <head> on various pages. These would include scripts.
  • Something like a <GoogleAnalytics /> that adds an analytics script.

In both of the above cases, while it's possible to insert that components multiple times, you would just never do so.

So there must be a case where you do want a component to be repeated but not all of its contents. What is the scenario?

@georges-gomes
Copy link
Contributor

Deduplication is not my primary use case.

Here is what I was thinking:

  • Transpile : I can write ES202X code and get a more compatible output.
  • Bundle : I can import bare modules from node_modules
  • Chunck : If modules are used on many pages, move them into a common-chunk.js
  • Minified

It's close to what you have magnificently done with CSS :D

@FredKSchott
Copy link
Member Author

After seeing the new Next.js Script component, what it can do, and the positive developer response to it, I'm now in favor of solving this that way.

  • We don't touch any <script> elements in your astro template, and ship directly to the browser
  • We add a Script component that can do all of these features that we'd like (ex: control load order, dedupe, minify/bundle or leave alone, etc.)

@jasikpark
Copy link
Contributor

I like the idea to dedupe <script type="module">, since it should only run once w/o needing a specific placement in the document (maybe it should just be deduplicated to the earliest instance in the document?

I use the fact that it runs once for some very sus "components" w/ my Hugo shortcodes:

@toinbis
Copy link

toinbis commented Jun 20, 2021

Very (literally - very!!) interested to see how this develops.

Yet my main are of interest would be to see astro components being rendered in server-side environment, i.e. in express.js app. Could we take server-side component rendering into account? Or is it something Astro would not be interested to pursue?

The core need is to be able to bundle multiple js snippets with Astro SFCs, also preferably setting some meta information for each of them. JS output composition would be handled in express.js codebase itself - be it inlining those scripts in request, or composing them in single file, or a mix of the multiple approaches.

Components paradigm in server-side (i.e. express, koa ) frameworks is arguably a huge area of web development that has been ignored for too long by webdev tooling. I'd be so glad to see Astro to filling this gap.

Currently I use two options(none of them very satisfying).

First approach:
a) You store react components in bit.dev npm storage/preview environment;
b) you orchestrate the components order in headless cms;
c) Express.js renders the nested component tree with props using rendertostaticmarkup;
d) You get your team members/clients to build landing pages themselves with 0 development involved. Super cool;
e) That works for html+css stack. What if you want to add some imperative dom manipulations written in vanilla js to certain components? You're just stuck;

Second approach:
a) instead of using react you push .njk template and some .js files as a component to bit.dev storage.
b) you are forced to write your custom i/o logic reading files, not too elegant at all.
c) Other devs do not favor such non-industry-confirmed approaches, etc. etc.

I honestly hope third approach will be just like the first one, just using Astro components instead, and besides .rendertostaticmarkup() which returns whatever is in the template there will be a possibility to extract all the piecies of vanilla js dom manipulation that belong to the component with .getComponentJsSnippetsArray() or so. Yes I understand that there are many conventions that you then need to follow in composing js in a backend, i.e. having all components unique id's and strictly scoping selector to very specific div, but that's a pleasant problems to solve.

I'd also be very glad to financially(at least to some extent) fund R&D of such feature development, if that only would make a difference.

@matthewp
Copy link
Contributor

@toinbis Thanks for commenting but what you are referring to seems a bit different from what this issue is trying to solve. Would you mind creating another issue so it can be discussed separately?

@toinbis
Copy link

toinbis commented Jun 23, 2021

Thanks, @matthewp, created separate issue here: #525 .

@matthewp matthewp mentioned this issue Jun 29, 2021
3 tasks
@FredKSchott FredKSchott changed the title Ability to transpile/bundle <script> tags inside of Astro components 馃挕 RFC: Ability to transpile/bundle <script> tags inside of Astro components Jun 30, 2021
@matthewp
Copy link
Contributor

A low-level solution to this problem with be making sure that Astro.request is available in all Astro components. Since Astro.request is unique to each page render you can use that as key to know if you've rendered something before. Pseudo-code:

---
const scriptsAdded = new WeakMap();

let includeScript = !scriptsAdded.has(Astro.request);
if(includeScript) {
  scriptsAdded.add(Astro.request);
}
---

{includeScript && <script>console.log('this is only added once');</script>}

However, this won't work due to the fact that this WeakMap will be created per-render :(

This would be nice however because it's not specific to scripts, you could use it to conditionally add any type of content.

@FredKSchott FredKSchott moved this from Needs Discussion to On Hold in 馃挕 RFC Tracker [No Longer Used] Jul 27, 2021
@jasikpark
Copy link
Contributor

I wonder if that would work for @tony-sull's svg use case? #706

@IanVS
Copy link
Contributor

IanVS commented Aug 5, 2021

what is the exact use case for having a script in a component in the first place?

I have a use case that I think could be interesting. I am making a <Dialog> astro component which uses the native html <dialog> element. This element requires a bit of javascript to attach listeners to other elements in order to open/close it. This seems to me to be well suited for a client script.

However, I'd also like this component to be generic, and for example, accept a query selector for the opener button that it should attach the listener onto. For now, it seems that there's no way to provide a prop to the client script, which is a bummer. But if there were a way, then I for sure wouldn't want the scripts de-duplicated. Oh, and I'd want to keep using type="module" too, so that I can import dialogPolyfill from '/dialog-polyfill.js' at the top of my script.

I think I can rejigger things so that I have a global script in public which exposes a class or function that accepts the parameters it needs, and I'll need to call it in the parent components which are importing my <Dialog>, rather than just providing the parameters to <Dialog> directly. This works, but is a bit of friction and breaks the component into a few different pieces instead of keeping all the logic in one place.

@jasikpark
Copy link
Contributor

I like the idea to dedupe <script type="module">, since it should only run once w/o needing a specific placement in the document (maybe it should just be deduplicated to the earliest instance in the document?

I use the fact that it runs once for some very sus "components" w/ my Hugo shortcodes:

https://github.com/jasikpark/jasik-xyz/blob/main/assets/js/iframe-with-chrome.js

I feel like it should work to auto-deduplicate scripts to their first addition into the page, because of these properties of them:
image

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Modules

馃挕 RFC Tracker [No Longer Used] automation moved this from On Hold to Completed Aug 27, 2021
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

Successfully merging a pull request may close this issue.

6 participants