Add Require and Import Values in `package.json` for better `ESM` Support #4490

thisjt · 2024-05-05T08:12:59Z

build/Bunder.mjs has been changed so that tabulator_esm output in dist/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

dominikg · 2024-05-05T09:07:07Z

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.

olifolkerd · 2024-05-05T11:41:13Z

@dominikg do you want to clarify this further? Possibly with any examples of areas of specific concern there?

olifolkerd · 2024-05-05T11:44:22Z

@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

thisjt · 2024-05-05T12:04:51Z

@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

pkg.module is used to output ESM, but pkg.exports is not defined. As NodeJS doesn't read pkg.module, the ESM output may be skipped. Consider adding pkg.exports to export the ESM output. pkg.module can usually be removed alongside too. (This will be a breaking change)

If the package isn't meant for Node.js usage, it is safe to ignore this suggestion, but it is still recommended to use "exports" whenever possible.
https://publint.dev/tabulator-tables@6.2.1

dominikg · 2024-05-05T13:58:20Z

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.

olifolkerd · 2024-05-05T14:22:21Z

@thisjt is the exports bit actually needed? If you point the modules option to the .mjs file does it work in sveltkit?

thisjt · 2024-05-05T15:38:02Z

@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 package.json looks like.

...
  "main": "dist/js/tabulator.js",
  "module": "dist/js/tabulator_esm.mjs",
  "sideEffects": [
    "**/*.css",
    "**/*.scss"
  ],
...

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 package.json looks like.

...
  "main": "dist/js/tabulator.js",
  "module": "dist/js/tabulator_esm.mjs",
  "exports": {
    ".": {
      "require": "./dist/js/tabulator.js",
      "import": "./dist/js/tabulator_esm.mjs"
    }
  },
  "sideEffects": [
    "**/*.css",
    "**/*.scss"
  ],
...

I'm not able to provide a stackblitz instance as they don't currently support import from a github repo.

olifolkerd · 2024-07-14T12:31:56Z

Hey All,

This will be included in todays patch release.

Cheers

Oli :)

lazka · 2024-07-14T19:15:02Z

fyi, this removed the export for the package.json file which made my build fail.

I used the path to package.json to figure out where the CSS is to copy that during bundling. I guess I can use the default import and go up the directory tree until I find the package.json instead. Other ideas welcome :)

[!] Error: Package subpath './package.json' is not defined by "exports" in /home/runner/work/app/app/node_modules/tabulator-tables/package.json
    at new NodeError (node:internal/errors:405:5)
    at exportsNotFound (node:internal/modules/esm/resolve:366:10)
    at packageExportsResolve (node:internal/modules/esm/resolve:713:9)
    at resolveExports (node:internal/modules/cjs/loader:590:36)
    at Function.Module._findPath (node:internal/modules/cjs/loader:664:31)
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:1126:27)
    at Function.resolve (node:internal/modules/helpers:188:19)

thisjt · 2024-07-14T19:53:19Z

fyi, this removed the export for the package.json file which made my build fail.

I used the path to package.json to figure out where the CSS is to copy that during bundling. I guess I can use the default import and go up the directory tree until I find the package.json instead. Other ideas welcome :)
[!] Error: Package subpath './package.json' is not defined by "exports" in /home/runner/work/app/app/node_modules/tabulator-tables/package.json
    at new NodeError (node:internal/errors:405:5)
    at exportsNotFound (node:internal/modules/esm/resolve:366:10)
    at packageExportsResolve (node:internal/modules/esm/resolve:713:9)
    at resolveExports (node:internal/modules/cjs/loader:590:36)
    at Function.Module._findPath (node:internal/modules/cjs/loader:664:31)
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:1126:27)
    at Function.resolve (node:internal/modules/helpers:188:19)

We might need to add this to package.json to fix the import css issue. I haven't really encountered this issue as I am importing the css file through node_modules (which is a bad practice I know, but I haven't bothered touching it since 2~ years ago)

"exports": {
    ".": {
      "require": "./dist/js/tabulator.js",
      "import": "./dist/js/tabulator_esm.mjs"
    },
    "./dist/*.css": {
      "import": "./dist/css/*.css",
      "require": "./dist/css/*.css"
    }
}

vitejs/vite#2657

olifolkerd · 2024-07-14T20:33:19Z

@thisjt Thanks for the headsup, ive just made an updated patch release on 6.2.3. let me know how that works for you.

Cheers

Oli :)

thisjt · 2024-07-15T14:53:48Z

fyi, this removed the export for the package.json file which made my build fail.

I used the path to package.json to figure out where the CSS is to copy that during bundling. I guess I can use the default import and go up the directory tree until I find the package.json instead. Other ideas welcome :)
[!] Error: Package subpath './package.json' is not defined by "exports" in /home/runner/work/app/app/node_modules/tabulator-tables/package.json
    at new NodeError (node:internal/errors:405:5)
    at exportsNotFound (node:internal/modules/esm/resolve:366:10)
    at packageExportsResolve (node:internal/modules/esm/resolve:713:9)
    at resolveExports (node:internal/modules/cjs/loader:590:36)
    at Function.Module._findPath (node:internal/modules/cjs/loader:664:31)
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:1126:27)
    at Function.resolve (node:internal/modules/helpers:188:19)

Can you confirm if this works on your end @lazka? Try editing node_modules/tabulator-tables/package.json and modify the exports to look like this

  "exports": {
    ".": {
      "require": "./dist/js/tabulator.js",
      "import": "./dist/js/tabulator_esm.mjs"
    },
	"./dist/*.css": {
		"import": "./dist/css/*.css",
		"require": "./dist/css/*.css"
	}
  },

Then try importing the css in your scss stylesheet like this

@import 'tabulator-tables/dist/tabulator_site.css';

Oli has not published ~~6.2.3~~6.2.4 yet, but this would be the expected edit for the export. Hopefully it will now work on your end.

@olifolkerd since ~~6.2.3~~6.2.4 is not yet out, would it be possible to also add this to the exports? It would be nice to be able to import scss stylesheets directly from the package and take advantage of modifying the scss variables.

  "exports": {
    ".": {
      "require": "./dist/js/tabulator.js",
      "import": "./dist/js/tabulator_esm.mjs"
    },
	"./dist/*.css": "./dist/css/*.css",
	"./src/scss/**/*.scss": "./src/scss/**/*.scss"
  }

So that the scss files can be imported~

// like this
@import 'tabulator-tables/src/scss/themes/tabulator_site.scss';

// instead of this
@import '../../node_modules/tabulator-tables/src/scss/themes/tabulator_site.scss';

The css and scss exports can be simplified as shown above as they are not affected on whether you're loading them in ESM, UMD, or CJS as per vitejs/vite#2657 (reply in thread)

Or maybe I can raise a new PR for this.

lazka · 2024-07-15T15:21:41Z

Can you confirm if this works on your end @lazka?

The package.json export is still missing in your example (as in "./package.json": "./package.json") , so I doubt it changes anything. And I'm not using SCSS.

Anyway, I solved it already via the workaround described above: digital-blueprint/toolkit@47d250d

So feel free to ignore my use case :)

Thanks for asking!

dominikg · 2024-07-15T18:59:01Z

as mentioned earlier, adding an exports map was a breaking change. not just for the reported package.json, but everything in src is now inacessible too.

should have published it as 7.0

olifolkerd · 2024-07-15T19:04:09Z

@dominikg could you explain what you are trying to access in src, none of those files are safe to include for production runs? At this point all the dist js and CSS files are available which should be all required for prod?

It would also be easy enough to add a wild card route for all files in src directory if needed

dominikg · 2024-07-15T19:17:58Z

I am not, nor am I suggesting it is a good idea. However it was possible in 6.2.1 and we all know not everyone follows best practices or follows suggestions. So it is very possible that at least some of your users cannot update to 6.2.2 without getting a build error and having to adjust their (admittedly not optimal) imports.

Unless you previously documented that src is not to be used, your users wouldn't have known just by looking at package.json of 6.2.1 it appears ok to use src.

Releasing this change as 7.0.0 and adding a changelog entry that describes how files that were accessible before are now no longer accessible would have been the more appropriate thing to do in terms of semantic versioning

olifolkerd · 2024-07-15T19:21:18Z

The same could be said if I change an internal variable name. But I don't expect people to do that and I don't update the major version for that either.

I would consider it a change in behaviour if it breaks expected standard documented behaviour which this doesn't, and if access to the src folder is important then I am more than happy to do a patch release with a wildcard export for that folder to make it accessible again making this a non breaking change :)

dominikg · 2024-07-15T19:23:39Z

It is your package and you know your users better than I do. I'd caution against adding a glob in exports map as they bring back the problem you just described yourself. every rename or delete of a file matched by that glob would be a breaking change again. So the exports map limited to dist gives you a lot more freedom internally, you should keep it that way.

require and import values for better mjs support

4004ab5

thisjt changed the title ~~Add Require and Import Values in package.json for better mjs Support~~ Add Require and Import Values in package.json for better ESM Support May 5, 2024

thisjt added 3 commits May 5, 2024 19:57

revert mjs back to js for bundler

4b29ed8

remove prettifier messing with format

8221489

revert mjs back to js for build compiler

07d958c

olifolkerd changed the base branch from master to 6.2.2 July 14, 2024 12:24

olifolkerd merged commit 50fdf6f into olifolkerd:6.2.2 Jul 14, 2024
1 check passed

thisjt mentioned this pull request Jul 15, 2024

SvelteKit 2.0 breaking "tabulator-tables" import statement sveltejs/kit#12185

Closed

jakebailey mentioned this pull request Jul 16, 2024

[tabulator-tables] Update for new export maps upstream DefinitelyTyped/DefinitelyTyped#70071

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Require and Import Values in `package.json` for better `ESM` Support #4490

Add Require and Import Values in `package.json` for better `ESM` Support #4490

thisjt commented May 5, 2024

dominikg commented May 5, 2024

olifolkerd commented May 5, 2024

olifolkerd commented May 5, 2024

thisjt commented May 5, 2024 •

edited

Loading

dominikg commented May 5, 2024

olifolkerd commented May 5, 2024

thisjt commented May 5, 2024 •

edited

Loading

olifolkerd commented Jul 14, 2024

lazka commented Jul 14, 2024 •

edited

Loading

thisjt commented Jul 14, 2024 •

edited

Loading

olifolkerd commented Jul 14, 2024

thisjt commented Jul 15, 2024 •

edited

Loading

lazka commented Jul 15, 2024

dominikg commented Jul 15, 2024

olifolkerd commented Jul 15, 2024 •

edited

Loading

dominikg commented Jul 15, 2024

olifolkerd commented Jul 15, 2024

dominikg commented Jul 15, 2024

Add Require and Import Values in package.json for better ESM Support #4490

Add Require and Import Values in package.json for better ESM Support #4490

Conversation

thisjt commented May 5, 2024

dominikg commented May 5, 2024

olifolkerd commented May 5, 2024

olifolkerd commented May 5, 2024

thisjt commented May 5, 2024 • edited Loading

dominikg commented May 5, 2024

olifolkerd commented May 5, 2024

thisjt commented May 5, 2024 • edited Loading

olifolkerd commented Jul 14, 2024

lazka commented Jul 14, 2024 • edited Loading

thisjt commented Jul 14, 2024 • edited Loading

olifolkerd commented Jul 14, 2024

thisjt commented Jul 15, 2024 • edited Loading

lazka commented Jul 15, 2024

dominikg commented Jul 15, 2024

olifolkerd commented Jul 15, 2024 • edited Loading

dominikg commented Jul 15, 2024

olifolkerd commented Jul 15, 2024

dominikg commented Jul 15, 2024

Add Require and Import Values in `package.json` for better `ESM` Support #4490

Add Require and Import Values in `package.json` for better `ESM` Support #4490

thisjt commented May 5, 2024 •

edited

Loading

thisjt commented May 5, 2024 •

edited

Loading

lazka commented Jul 14, 2024 •

edited

Loading

thisjt commented Jul 14, 2024 •

edited

Loading

thisjt commented Jul 15, 2024 •

edited

Loading

olifolkerd commented Jul 15, 2024 •

edited

Loading