Reload scripts

[OliverSpeir]

Changes

• What does this change?

Adds ability to add data-astro-reloaddata-astro-rerun to inline scripts so they will be re-executed after view transition

Issue #9798 is related, in it both Mathew and Martin agreed this would be acceptable change

Testing

Didn't add tests because this is very small change, but happy to add if someone thinks its needed

Docs

This should be documented
/cc @withastro/maintainers-docs

[changeset-bot]

🦋 Changeset detected

Latest commit: 1d667f8

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

[martrapp]

@OliverSpeir, I'm sure you are aware that adding data-astro-reload turns TypeScript modules into failing inlined scripts.

<script>
  (function (a: string) {
    console.log(a);
  })("iife test");
</script>

... works just fine with Astro. After adding data-astro-reload to the script it only outputs

This could be very confusing for users. In fact, we can't reload bundled module scripts anyway.
To be precise: a script with src attribute and type="module" can not be reloaded, all others can. This is independent of whether the script is inline or bundled. Astro turns scripts that are not inlined, i.e. bundled, into scripts of type="module" with a src attribute. So these can not be reloaded.

Imho we should better warn users when they try to reload a bundled module script. What do you think?

Up to now, we do not have error messages in the browser. Is there a chance to detect this server side?

[OliverSpeir]

(function (a: string) {
console.log(a);
})("iife test");

I thought about this, adding a check for this is possible and probably could be done for both is:inline as well. I'm not sure where I'd get started on adding that though, I assume we'd want that printed in the terminal, it could even throw an astro error

Do you think there's a way we could inform the user better that adding this property makes it an is:inline script because currently that script will create the browser error with is:inline without informing the user either so what I think would be good enough for now, or at least not worse than what we had before would be clearly letting the user know this is only for is:inline scripts and will also serve the exact same purpose as the is:inline attribute if you don't add that

@martrapp

[martrapp]

At this time of day, my brain is too slow. You are correct that any attribute other than src makes an astro script inline. There is an explicit warning in the documentation. Also, with the current settings it's probably impossible to force astro to output a script with src && data-astro-reload && type="module", which is the only case where we can't reload.

So yes, it's not an astro-data-reload problem, it's an auto-inlining/bundling-is-confusing-and-may-break-things problem.
I just remember a discussion on discord the other day that we're thinking about making exceptions, like for type="module" that don't trigger inlining. But we can worry about it when data-astro-reload will no longer trigger inlining.

[OliverSpeir]

it's probably impossible to force astro to output a script with src && data-astro-reload && type="module", which is the only case where we can't reload.

Ah this is something I forgot about, I should check this, does the src need to be remote for this condition? I honestly forgot about <script type="module" data-astro-reload> when writing the docs for this so I should reread them and make sure that's clear. I did test that <script type="module" data-astro-reload> will re-load

So yes, it's not an astro-data-reload problem, it's an auto-inlining/bundling-is-confusing-and-may-break-things problem. I just remember a discussion on discord the other day that we're thinking about making exceptions, like for type="module" that don't trigger inlining. But we can worry about it when data-astro-reload will no longer trigger inlining.

Yeah this will be something to be aware of for sure, although

Also something I just thought about is there's intellisense auto complete for is:inline on script tags, I'm not sure where that comes from but that should probably happen for this attribute

[martrapp]

Sorry for blocking, but I'm having doubts about the name right now. I would like to discuss with Erika/Matthew whether it is perfect or not so good to use reload as a name for both opt-in on re-execution of scripts and opt-out from view transitions on links.

Pro for same term:

• less cluttered name space
• no risk of confusion: opt-out only for links, re-execution only for scripts

Contra:

• not so simple to describe, e.g. in VS-Code popups "on , , and : opt out from view transitions, on <script> elements opt-in to re-execution during view transitions"

[OliverSpeir]

I see your point that makes sense! Never thought of it like that, definitely happy to change it to anything else.

I went with data-astro-reload because you opt out of view transitions' default behavior with it, but I'm not tied to it.

[matthewp]

Yeah I think it makes sense to give this a different name given that it's not the same thing. data-astro-reexecute or something like that (hopefully we can think of something better than that!)

[matthewp]

@bluwy brought up a good point, do we know if scripts (I'm thinking) 3rd party that really need this? Or do we have support threads of people who needed it? Although it sounds fine, and like something people might run into, I'm not super comfortable with releasing a feature until we know for sure that people have ran into a problem.

[OliverSpeir]

@bluwy brought up a good point, do we know if scripts (I'm thinking) 3rd party that really need this? Or do we have support threads of people who needed it? Although it sounds fine, and like something people might run into, I'm not super comfortable with releasing a feature until we know for sure that people have ran into a problem.

It's hypothetical, I mean I'll use it but I could also work around it. I'm sure there are some 3rd party packages out there though.

In the initial issue @matthewp and @martrapp agreed to it, and thats why I submit this PR. I'll understand of course if we decide not to add it, but given that this is restoring a previous functionality of Astro's I think it would be nice to have. Polluting the window, and not having functions execute after every transitions unnecessarily seems like a valid use case to me. I haven't done the math, but wouldn't there be some concern about consuming memory or slowing down mobile devices if they have transitioned to many pages on a site (and thus accumulated many scripts listening for life cycle events)?

[brandonedley]

@bluwy brought up a good point, do we know if scripts (I'm thinking) 3rd party that really need this? Or do we have support threads of people who needed it? Although it sounds fine, and like something people might run into, I'm not super comfortable with releasing a feature until we know for sure that people have ran into a problem.

It's hypothetical, I mean I'll use it but I could also work around it. I'm sure there are some 3rd party packages out there though.

In the initial issue @matthewp and @martrapp agreed to it, and thats why I submit this PR. I'll understand of course if we decide not to add it, but given that this is restoring a previous functionality of Astro's I think it would be nice to have. Polluting the window, and not having functions execute after every transitions unnecessarily seems like a valid use case to me. I haven't done the math, but wouldn't there be some concern about consuming memory or slowing down mobile devices if they have transitioned to many pages on a site (and thus accumulated many scripts listening for life cycle events)?

Im currently running into this issue. Need to include an iframe on my site. Iframe is given to me by a 3rd party. 3rd party gives me a script to include that finds an link they provided and injects iframe. Then it uses iframe resizer to size the iframe. Without this fix, if a user is on the page that contains the iframe and then clicks on a link that takes them to the same page, the script does not load.

[matthewp]

We discussed it and are fine with releasing this feature. We want a different name though. I think presently the best we've thought of is data-astro-rerun.

[OliverSpeir]

rerun is nice 👍 will do

[martrapp]

It's good to give different things different names.

My main motivation for #10094 and withastro/compiler#965 was to improve errors/warnings to inform users that (what is now called) data-astro-rerun will not work with external ESM scripts. I plan to close those, but i still would like to see some way to warn users.

@matthewp & @bluwy: What would be the best approach to just error when data-astro-rerun is set on a <script> which has its src attribute defined and type="module"?

[OliverSpeir]

One nice piece of this could be a linting rule @martrapp

Similar to this one: withastro/compiler#970

[martrapp]

One nice piece of this could be a linting rule

Hello Oliver, thank you very much for pointing this out! I wasn't aware of withastro/compiler#970. The warning from withastro/compiler#965 could well belong there!

[bluwy]

LGTM! I forgot to reply here, but yeah I've talked with Matthew that the usecase you explained makes sense to me.

[martrapp]

LGTM!

[MoustaphaDev]

LGTM!

[natemoo-re]

Blocking until minor release merge day since GitHub Actions isn't working on the fork 😭

[sarah11918]

Docs approves!

Merge day