search

tscanlin / tocbot

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

Add ESM version #338

Closed bashmish closed 2 months ago

bashmish commented 2 months ago

Fixes #337

tscanlin commented 2 months ago

Hey, thanks for the PR! This looks good to me. How would this affect imports of other sub-dependencies for non-esm projects? I guess since utils wasn't touched it shouldn't be a big deal.

Thanks again!

bashmish commented 2 months ago

Hey, thanks for the PR! This looks good to me. How would this affect imports of other sub-dependencies for non-esm projects? I guess since utils wasn't touched it shouldn't be a big deal.

Thanks again!

It shouldn't impact subdependencies for non-ESM, I didn't change the behavior for those.

Do you have some project where we can test if this works? I can make a prerelease version with @bashmish/tocbot for now to be able to test it on a real project.

bashmish commented 2 months ago

Should there be a test for .cjs.js require style of use?

@tscanlin static/js/tocbot.js is generated with the input file of index-dist.js with a UMD wrapper and is injected like before via a script tag, so I thought that would be sufficient to test it like that, despite that test cases are loaded from a ESM. The latter is done, because it was easy to use such file to load in both ESM and UMD versions and reuse tests, it's only the tests cases which are loaded like that, the initialisation of the library remained the same I think for UMD, and for ESM there is a separate new initialisation file. Maybe I didn't get some nuances in how UMD was tested before, would be nice if you can elaborate.

I don't currently have a project that is using it aside from in the project itself. But running the local development server and requiring from there may help to test it.

What particular way of loading do you want to test? I used ESM and a simple script. ESM is standard, injecting a script via a script tag and exposing on the window.tocbot is also standard (this was the way it was tested before my PR), no special tools are involved. Then there would be more ways to load it using different tools, with a spectrum between old-school require.js and new-school like Vite.

tscanlin commented 2 months ago

Thanks for the PR!!

tscanlin commented 2 months ago

tocbot@4.27.0 is published with this change. Thanks @bashmish!