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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should we add a script load event ? #498

Closed
JulienKode opened this issue Mar 24, 2020 · 8 comments
Closed

Should we add a script load event ? #498

JulienKode opened this issue Mar 24, 2020 · 8 comments

Comments

@JulienKode
Copy link

Thank you for this nice library

Currently it's possible to trigger a load event on a image:
Example:

const { getByTestId, debug } = render(<Stuff/>);

fireEvent.load(getByTestId('my-stuff'));

expect(stuff).toHaveBeenCalledWith(true);

But is there any way to do that with script ?
Should we implement it here

events: ['load', 'error'],

@JulienKode JulienKode changed the title Should we add an script load event ? Should we add a script load event ? Mar 24, 2020
@kentcdodds
Copy link
Member

What would it take to support this? It's it not already supported?

@gertsonderby
Copy link

I'm digging a little. This test fails (using react testing lib):

test('can load a script', () => {
    const handler = jest.fn();
    render(<script onLoad={handler} src="https://example.com/test.js" />);
    const script = document.querySelector('script');
    expect(handler).not.toHaveBeenCalled();
    fireEvent.load(script);
    expect(handler).toHaveBeenCalled(); // Was not called.
});

But this one succeeds:

test('can load a script', () => {
    const handler = jest.fn();
    render(<script src="https://example.com/test.js" />);
    const script = document.querySelector('script');
    script.onload = handler;
    expect(handler).not.toHaveBeenCalled();
    fireEvent.load(script);
    expect(handler).toHaveBeenCalled();
});

As you can see, the only difference is that the handler is set indirectly. So there may be an issue wrt. setting the handler?

@JulienKode
Copy link
Author

@gertsonderby I didn't notice that if I set the onload directly in the script it work

@kentcdodds
Copy link
Member

Sounds like this may be an issue with React's support of onLoad?

@gertsonderby
Copy link

Possibly. Although I don't think they're all too interested in supporting script tags like that, it seems out of scope.

Personally, I would add them to <head> with auseEffect hook instead of rendering them directly.

@sibelius
Copy link

is this a workaround to test jQuery onload like this code ?

$(() => {
   console.log('testing lib onload jquery')
})

@danascheider
Copy link

danascheider commented Jun 26, 2022

Hi everybody, I'm commenting in hopes of saving somebody the full week of frustration I had with this. I was testing a useEffect hook using renderHook. The hook adds a <script> tag to the document head in a useEffect hook, like @gertsonderby suggested. I had the following test for my hook:

it('calls the success callback', async () => {
  const params = { onSuccess, clientId, staySignedIn: true }

  // This hook calls the function that adds the script to the document
  renderHook(() => useGoogleLogin(params))

  const script = document.querySelector('script')
  fireEvent.load(script)

  await waitFor(() => expect(onSuccess).toHaveBeenCalled())
})

I was getting the same issue - if I set onSuccess to be the onload callback within the test itself (script.onload = onSuccess), it worked, and it failed if I didn't do that explicitly.

This is the function called in the hook:

const loadGapiScript = (document, src, onLoad, onError) => {
  const script = document.getElementsByTagName('script')[0]

  let js = document.createElement('script')
  js.id = 'google-auth'
  js.src = src
  js.async = true
  js.defer = true

  js.onerror = onError
  js.onload = onLoad

  if (script && script.parentNode) {
    script.parentNode.insertBefore(js, script)
  } else {
    document.head.appendChild(js)
  }
}

The following two lines turned out to be the problem:

js.onerror = onError
js.onload = onLoad

Changing them to this ended up solving the issue:

js.onerror = e => onError(e)
js.onload = e => onLoad(e)

So, to use the original example, this should work:

render(<script onLoad={e => handler(e)} src='https://example.com/test.js' />)

Unfortunately I'm not sure why this solved my problem, but it did 🤷‍♀️ If anybody knows why this would be I'd love to know,.

@eps1lon
Copy link
Member

eps1lon commented Jul 9, 2022

Firing load events at scripts is supported in @testing-library/dom. If you find issues specific to @testing-library/dom please provide a minimal repro (e.g. no rendering with React) and open a new issue.

For people looking for support in React: Scripts will not execute if rendered via React so it's expected that there's no load event: https://codesandbox.io/s/react-script-does-not-work-the-way-you-think-it-does-stc66c?file=/src/index.js. Check how your component behaves in a browser first before writing tests that replay the expected behavior.

So you need to manually insert the script into the DOM e.g. like in #498 (comment)

@eps1lon eps1lon closed this as not planned Won't fix, can't repro, duplicate, stale Jul 9, 2022
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

No branches or pull requests

7 participants