tscanlin / tocbot

Build a table of contents from headings in an HTML document.
MIT License
1.37k stars 114 forks source link

ESM version #337

Closed bashmish closed 2 months ago

bashmish commented 2 months ago

Do you have plans to support an ESM module output? As "module": "path/to/esm-file.js" or via exports in package.json?

I'd be happy to work on it and discuss the best approach to keep things maximum compatible or introduce a new major if needed.

tscanlin commented 2 months ago

Hey, this is a great idea, I'm definitely open to adding better ESM module support. I just hadn't gotten around to it yet. If you'd like to work on this I'd very much appreciate a PR. This blog post has some helpful info for supporting both types in one repo, but I am also open to other approaches. I guess I would just prefer not to break backwards compatibility. https://the-guild.dev/blog/support-nodejs-esm#exports-field

Thank you!

bashmish commented 2 months ago

Great, I'll check the post and work on this soon. The usage of exports might be tricky since it will block access to certain private files internally, but people might be using them, but ideally I'd like to use that as it gives simple conditional configuration for ESM and CJS.

Is tocbot used directly in node, or it's only for browsers?

tscanlin commented 2 months ago

Ok sounds good. It is only used in browsers, not in node. Yeah, hopefully the private files can be declared with:

"./*": {
      "require": "./dist/*.js",
      "import": "./dist/*.mjs"
}

But I'm not sure how that would affect deeply nested files. Anyway, I think the files in utils would be the main ones people import directly. Thanks!

bashmish commented 2 months ago

I made some progress, now working on tests, hope to finish this weekend.

This is my current exports API

  "exports": {
    ".": {
      "require": "./dist/tocbot.cjs",
      "import": "./dist/tocbot.mjs"
    },
    "./styles.css": "./dist/styles.css",
    "./tocbot.css": "./dist/tocbot.css",
    "./dist/tocbot.js": "./dist/tocbot.js",
    "./dist/tocbot.min.js": "./dist/tocbot.min.js",
    "./dist/styles.css": "./dist/styles.css",
    "./dist/tocbot.css": "./dist/tocbot.css"
  },

The files in dist should be exported in the same way as before, so that people who import tocbot/dist/tocbot.js can have a fully compatible update.

For the bare module I'll export files too, but the build will be different. I was thinking of doing both CJS and ESM on the root, but if you say it's only for browsers, I can as well just remove the CJS one, which would simplify things even further, but in case someone needs a CJS one, we can add it later, it's a new entry point anyway, so should be considered a new feature then.

"./*": {
      "require": "./dist/*.js",
      "import": "./dist/*.mjs"
}

Paths like tocbot/tocbot.js and alike are new too, can add it if you want, but they are not currently available, so it's also a new feature then. For styles I decided to add them right away, so that people who wanna use new ESM-compatible entry points will be able to write simple import without the dist and because I consider dist to be non-public for ESM.

UPDATE:

On a second thought. There is currently "main": "src/js/index.js",, which basically points at the sources, so people who import the bare module like import { init, destroy } from "tocbot" and alike often end up there. It's also the one that I currently get when building my code with rollup, and it's causing bugs, as it's hard to use the UMD like that. When exports will be defined, most tools will ignore the package.json main and use exports instead, which is technically not a backwards compatible update, but in reality the tools which support exports are better off using that instead of sources. I think the vast majority of user will either use tocbot/dist/{tocbot,tocbot.min}.js (which I make fully compatible) or will use exports and benefit from the new behavior, despite it changes.

bashmish commented 2 months ago

I realised also that ./dist/tocbot.mjs is not needed if the sources are ESM (which is required to make it work for my ESM use-cases anyway). So we can as well keep the bare module path to point at the old src/js/index.js also in the exports, even for more compatibility. I think this is gonna be better also when debugging the code since the sources will be just loaded in DEV mode without any compilation required, which is one of the selling point of ESM in general.