Broken type definition in focus-trap-react 11

[Mathias-S]

Description
I am using ESM-style imports and exports in my project.

The change from export = FocusTrap to export declare class FocusTrap in index.d.ts means that it is now only possible to use named imports, i.e. import { FocusTrap } from "react-focus-trap" (according to the types).

However, this is incompatible with module.exports = FocusTrap; in src/focus-trap-react.js, which only lets you do default imports, i.e. import FocusTrap from "react-focus-trap".

So the type change needs to be reverted so only default imports works, or a named export should be provided instead:

exports.FocusTrap = FocusTrap;

But that would obviously also be a breaking change. It is technically possible to hack it and provide both a named and default export with CJS:

module.exports = FocusTrap;
module.exports.FocusTrap = FocusTrap;

Versions

• focus-trap-react version used: 11.0.0

Sandbox
Minimal reproduction in CodeSandbox should display the type error in the editor: https://codesandbox.io/p/sandbox/5fz5r4

Local repro

npm create vite@latest focus-trap-test -- --template react-ts
cd focus-trap-test
npm install focus-trap-react
# then edit `src/App.tsx` to add FocusTrap. If you have the TS language server running, your editor should highlight the error

[stefcameron]

@Mathias-S 🤦‍♂️ Thank you for being quick to file this and help me out with a PR to correct it.

I believe my actual intent (months ago when I prepared the changes ahead of React 19's official release) was to move away from the default export since Tabbable and Focus-trap exclusively use named exports and I generally feel that's a better way to go for bundlers. So I figured I'd use the breaking change opportunity of v11 to move to that, but completely missed adjusting the code after adjusting the types, and also mentioning it in the changelog which I prepared just before publishing.

I guess I will kick that can down the road again since I kind of missed the major version opportunity at this point. Introducing what would technically be another breaking change in a patch to fix the code to match the types doesn't feel great, and neither does doing back-to-back major releases. 🫤

[Mathias-S]

I agree that named exports are better than default exports. Maybe a temporary solution would be to provide both?

And then also mark the default export as deprecated using JSDoc /** @deprecated */ and update the docs? Then the migration to using named exports will be less painful for apps using this package.

[stefcameron]

@Mathias-S I like that idea! At least it's progress toward the goal as I (ever so) slowly refactor this library. I'd love to have it be 100% TypeScript some day instead of have "tangential" types like it does today.

[stefcameron]

@Mathias-S I have opened #1406 to do as you suggested (and closed #1397): Provide both exports and mark the default export as deprecated. I tested this in your local vite repro and it works as intended. Please have a look and LMK if I missed something -- particularly with the types.

[stefcameron]

@all-contributors add @Mathias-S for bug, review

[allcontributors]

@stefcameron

I've put up a pull request to add @Mathias-S! 🎉