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

fix: ensure fireEvent is exported #339

Merged
merged 1 commit into from
Mar 25, 2024

Conversation

mcous
Copy link
Collaborator

@mcous mcous commented Mar 17, 2024

Overview

#328 moved exports from pure.js to index.js and switched everything to wildcard exports:

export * from './pure.js'
export * from '@testing-library/dom'

If two wildcard exports emit the same export, neither will be included.

There is also export * from "mod", although there's no import * from "mod". This re-exports all named exports from mod as the named exports of the current module, but the default export of mod is not re-exported. If there are two wildcard exports statements that implicitly re-export the same name, neither one is re-exported.

This means the current release on next is not exporting fireEvent. This PR moves the STL exports back to named exports to resolve the bug

Observing the bug

pnpm add -D svelte @testing-library/svelte@next
node
> const stl = await import('@testing-library/svelte')
> stl.fireEvent
undefined

Why didn't tests catch this?

Vite/Vitest appears to resolve exports differently from local files compared to node_modules. Since the test suite imports directly from src/index.js, the behavior is slightly different:

  • The conflicting wildcard export of fireEvent is available
  • It is set to whatever the last export was

In our case, since export * from '@testing-library/dom' is listed second, that means the tests are running with the vanilla fireEvent, and coincidentally passing because a no-op await in the test was enough of a wait for Svelte to flush pending changes.

Without a more sophisticated test suite that packs and installs the library into example projects (which is a good long term idea!) this particular problem is hard to guard against with unit tests.

Change log

  • Update exports, with comments so we don't forget that the explicit named exports are important
  • Update the tests to ensure fireEvent is returning a Promise
  • Fix import/export sorting lint errors in files I was touching and looking at

@mcous
Copy link
Collaborator Author

mcous commented Mar 25, 2024

@yanick did you see this one? Relatively bad regression on the next tag

@yanick
Copy link
Collaborator

yanick commented Mar 25, 2024

yup. was waiting for the other branch to be merged.

@yanick yanick merged commit 40feeb4 into testing-library:next Mar 25, 2024
16 checks passed
Copy link

🎉 This PR is included in version 4.2.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

None yet

2 participants