next/script onLoad run once per cache Key (id, src)

[bordeo]

What version of Next.js are you using?

12.0.2

What version of Node.js are you using?

14.18.1

What browser are you using?

Chrome

What operating system are you using?

macOS

How are you deploying your application?

codesandbox

Describe the Bug

When using more than one Script with same id or src, only first script that complete load trigger the onLoad prop.
Same problem happens using one Script but after unmount and remount, for example when navigating throughout client-side router the onLoad will not be called.
@giorgiabosello

Expected Behavior

Script component should load once the src script but run onLoad on component did mount and script is loaded.

To Reproduce

https://codesandbox.io/s/nextjs-script-onload-ltm95

[havran]

I just need same feature (fire onLoad every time, when script is added to attach some event listener). Any news here?

Meanwhile I use hook:

import { useEffect, useState } from 'react';

type Callback = () => void;

interface ScriptAttributes {
  id: string;
  src: string;
  async: 'async';
  defer: 'defer';
}

function setAttributes<T>(el: Element, attributes: T): void {
  Object.keys(attributes).forEach((key: string) => {
    el.setAttribute(key, attributes[key]);
  });
}

function addScript(scriptUrl: string, elName: string, callback: Callback | undefined): void {
  const elScriptId = `${elName}-script`;
  const elScriptPresent: HTMLElement | null = document.getElementById(elScriptId);

  // Avoid multiple inserting of script element.
  if (elScriptPresent) {
    if (callback) {
      callback();
    }
    return;
  }

  const elScript: HTMLScriptElement = document.createElement('script');

  if (callback) {
    elScript.onload = () => callback();
  }

  setAttributes<ScriptAttributes>(elScript, {
    id: elScriptId,
    src: scriptUrl,
    async: 'async',
    defer: 'defer',
  });
  document.body.appendChild(elScript);
}

function useScript(name: string, src: string): boolean {
  const [loaded, setLoaded] = useState<boolean>(false);
  const callback: Callback = () => setLoaded(true);

  useEffect(() => {
    addScript(src, name, callback);
  });

  return loaded;
}

export default useScript;

And usage:

  // inside component where is script loaded
   const loaded = useScript();

   useEffect(() => {
    if (elementRef.current && loaded) {
      elementRef.current.addEventListener('someevent', (e) => ...);
    }
  }, [loaded]);

[giorgiabosello]

I just need same feature (fire onLoad every time, when script is added to attach some event listener). Any news here?

Meanwhile I use hook:

import { useEffect, useState } from 'react';

type Callback = () => void;

interface ScriptAttributes {
  id: string;
  src: string;
  async: 'async';
  defer: 'defer';
}

function setAttributes<T>(el: Element, attributes: T): void {
  Object.keys(attributes).forEach((key: string) => {
    el.setAttribute(key, attributes[key]);
  });
}

function addScript(scriptUrl: string, elName: string, callback: Callback | undefined): void {
  const elScriptId = `${elName}-script`;
  const elScriptPresent: HTMLElement | null = document.getElementById(elScriptId);

  // Avoid multiple inserting of script element.
  if (elScriptPresent) {
    if (callback) {
      callback();
    }
    return;
  }

  const elScript: HTMLScriptElement = document.createElement('script');

  if (callback) {
    elScript.onload = () => callback();
  }

  setAttributes<ScriptAttributes>(elScript, {
    id: elScriptId,
    src: scriptUrl,
    async: 'async',
    defer: 'defer',
  });
  document.body.appendChild(elScript);
}

function useScript(name: string, src: string): boolean {
  const [loaded, setLoaded] = useState<boolean>(false);
  const callback: Callback = () => setLoaded(true);

  useEffect(() => {
    addScript(src, name, callback);
  });

  return loaded;
}

export default useScript;

And usage:

  // inside component where is script loaded
   const loaded = useScript();

   useEffect(() => {
    if (elementRef.current && loaded) {
      elementRef.current.addEventListener('someevent', (e) => ...);
    }
  }, [loaded]);

Any words from Next. We also use a hook based on this one and it's working perfectly.

[housseindjirdeh]

This is indeed an issue, so thanks all for flagging this.

The hook is a workaround for now, but I'll either approve @bordeo's PR (#30966) to remove the current caching functionality or investigate a way to improve it so that it doesn't ignore onLoad for repeat scripts.

[bordeo]

@housseindjirdeh can I help doing something about this issue?

[eps1lon]

I think it's technically correct to call onLoad only once because we only get the load events once. That's what we want in the end.

It seems safer to add a new event handler that fires once the script is ready. So once it basically fire with the load event and otherwise it fires at some other point (effect seems not ideal though).

For our use case specifically we were loading a 3rd party library. So we started out with a "pending" state and flipped that to "resolved" in onload. Now we immediately start with resolved if we know that the 3rd party library is available. And for <Script /> loading simultaneously we're attaching onload to the actual <script /> HTML element instead of using <Script onLoad />.

There are still some race conditions we might be missing though.

[bordeo]

I can agree with you that onLoad could run once but in this case an onReady props is mandatory to handle multiple use of the same 3rd party library.

In my experiences I didn't find a case where I need an onLoad event to run once.

[eps1lon]

In my experiences I didn't find a case where I need an onLoad event to run once.

Sure. But that's a documentation issue i.e. use onReady in the guide not onLoad. When you repurpose the onLoad handler you risk confusing people that know the load event and expect Next.js to de-duplicate scripts.

[housseindjirdeh]

Sorry for the delay here again folks.

I actually misread the issue the first time around and thought the problem was that onLoad was only running for a single Script even when multiple Script components are used on a page with different src attributes. That's thankfully not the behavior and it works fine in that regard.

I actually agree with @eps1lon here. If a single script is loaded once, then I wouldn't expect onLoad to fire more than once. Anything otherwise seems a little confusing 🤔

@bordeo One more question: Why are you using more than one Script with the same id and src? If I understand your use case a bit more, maybe we can find a different solution that can work for you.

[zacharyliu]

While I don't disagree that firing onLoad once is technically correct, I think that this ends up being too limiting in real use cases. My use case is to load a third-party JS widget and then run some initialization code after it's ready. A naive implementation using onLoad would work the first time the component is mounted but not the second time.

What about adding an onLoadingComplete event that always fires, like next/image? https://nextjs.org/docs/api-reference/next/image#onloadingcomplete

[bordeo]

Thank you @housseindjirdeh for your reply.
I'm using Script to load third-party JS for example "Klarna On-site messaging" or "Google Maps".
I want to use more than one "Klarna message" on a page and more than one "google maps widget".
To achieve that I use Script inside a component to render for example a Google Maps and I use that component more than once on a page without thinking if Script is loaded or not because Script would take care of anything.
At the actual state to keep track of if Script is already loaded in another component I must save it in a persistent Global State the first time it loaded.

But the problem manifests itself even when one Script is used and the component that uses it un-mount and re-mount for example when changing route client-side.

I don't think all this is correct behavior. Maybe firing an onReady event could be the right way to solve this issue.
I didn't find a use case where onLoad that fired once is useful.

[housseindjirdeh]

Thanks for the details @zacharyliu and @bordeo. I think I fully understand the concern now.

I agree that it would probably make more sense to have a separate prop for this (e.g. onReady or onMount). The easiest solution would be to throw this in a useEffect so that it runs every time the component is mounted:

useEffect(() => {
  if (onReady) {
    onReady()
  }
}, [onReady])

There's no guarantee that this would run before onLoad though. Is that an issue? Or would it be better if onReady ran at the same time as onLoad for the first time and then after the component mounts after that?

[zacharyliu]

I think it would need to run at the same time as onLoad in order for it to be useful as a way to run code that depends on the script. On subsequent mounts, it should run only if the script successfully loaded. It should also handle race conditions, like if the Script is unmounted before it finishes loading the first time.

As a more concrete example, the Google Maps JS SDK requires running new google.maps.Map(...) after the JS script finishes loading. (The SDK's built-in callback doesn't work well in a Next.js environment since it relies on a global function.) If onReady were called before the script is actually loaded, then the method call would fail. Demo: https://codesandbox.io/s/withered-currying-zqre3d?file=/pages/index.js

[bordeo]

@zacharyliu explains very well.

It would be better if onReady ran at the same time as onLoad or after (never before) for the first time and then after the component mounts after that and is successfully loaded before.
Race conditions could occur easily when a user navigates to a page and suddenly click on a Link before Script is fully loaded. In this case, the component unmount before onReady is triggered.

[housseindjirdeh]

Thanks for brainstorming with me folks, I appreciate it :)

PR #38849 should solve for this by introducing onReady as a prop. Please feel free to leave any comments directly in the PR if you think anything should be changed, but it's set up to run after load for when the script first loads and then again with every component re-mount.

[Zerebokep]

Can't wait for it. :)

[github-actions]

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.