-
Notifications
You must be signed in to change notification settings - Fork 782
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
Add Require and Import Values in package.json
for better ESM
Support
#4490
base: master
Are you sure you want to change the base?
Conversation
package.json
for better mjs
Supportpackage.json
for better ESM
Support
thanks for working on improving esm compatibility ❤️ Please keep in mind that adding an exports map is almost always a breaking change and you have to list all exports that are needed. |
@dominikg do you want to clarify this further? Possibly with any examples of areas of specific concern there? |
@thisjt could you revert the change to the bundler please and only include the e package change. Plenty of people still use the .js file directly when pulling from cdns so the .js file cannot be removed without disabling a huge number of users. Instead I will make the bundler duplicate the output to the new extension and deprecate the .js and remove it at the next major version change in a year or two to give people a chance to adapt |
@olifolkerd done. You may review the changes and then take over from there. publint references this error as a breaking change when pkg.module is removed. I think this is not necessary as this is a frontend package and publint says that it's fine to ignore it if not used in a NodeJS environment. Maybe dominik can expound further
|
I don't know the details behind the current structure of this package, just provided links to information how to make this esm compatible. Most ways to do this are a breaking change due to the way it changes how exports of your package can be reached. Without an exports map, everything that is exported can be imported by users. With an exports map, only the entries in the exports map can. publint and the guide by antfu go into more detail about this. |
@thisjt is the exports bit actually needed? If you point the modules option to the .mjs file does it work in sveltkit? |
@olifolkerd yes I assume it's required. This repo (https://github.com/thisjt/tabulator-import-issue/tree/rename-import) imports (https://github.com/thisjt/tabulator-sveltekit2fix/tree/tabulator-import-rename). This throws an error. This is what its
This repo (https://github.com/thisjt/tabulator-import-issue/tree/rename-import-with-exports) imports (https://github.com/thisjt/tabulator-sveltekit2fix/tree/tabulator-import-with-exports). This works fine and the table is being rendered correctly. This is what its
I'm not able to provide a stackblitz instance as they don't currently support import from a github repo. |
build/Bunder.mjs
has been changed so thattabulator_esm
output indist/js
will have the.mjs
extension.package.json
has been changed so that the file that the bundler creates is properly referenced.Referencing bug report: #4378