theKashey / react-focus-on

🎯 Solution for WAI ARIA compatible modal dialogs or full-screen tasks, you were looking for
MIT License
336 stars 14 forks source link

Mixed module semantics breaks rollup build #41

Closed marvinhagemeister closed 4 years ago

marvinhagemeister commented 4 years ago

Although the source is authored with ES-Modules, there is a require statement inside Combination.tsx. Mixing both import statements and require statements is invalid according to the spec.

https://github.com/theKashey/react-focus-on/blob/7640331427f136d27dd856cdb43f4df5fbcb2969/src/Combination.tsx#L7

It seems to me that this code should load the component lazily and makes use of a webpack specific behaviour which will load that code lazily. Nowadays it probably should use the standardized import() call instead.

FYI: There is a workaround for the invalid portion by setting the transformMixedEsModules: true for @rollup/plugin-commonjs, but that just turns it into a synchronous import.

theKashey commented 4 years ago

The idea behind it was to postpone module executing, still having it in the bundle.

Only possible with:

As long as both ways are not an option for a library - I've chosen this way.


Actually I was thinking about providing another entrypoint with the dynamic import used inside, as long as that's actually the goal, however it's also something not everyone could afford.

Please give me some time to check what and why @rollup/plugin-commonjs expects from the code, and is there any proper solutions for this problem.

theKashey commented 4 years ago

Decided to use plain static import after conversation with @TrySound.