Skip to content
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

Rewrite prism in esm #2715

Closed
hyrious opened this issue Jan 17, 2021 · 13 comments
Closed

Rewrite prism in esm #2715

hyrious opened this issue Jan 17, 2021 · 13 comments
Milestone

Comments

@hyrious
Copy link

hyrious commented Jan 17, 2021

Motivation
Many modules are supporting esm now, it is better for bundlers to do tree-shaking and do care node.js and browser environment with separated bundle.js.

Description
Bundlers like rollup or webpack want to see esm modules and they do tree-shaking for smaller size. Currently you're using cjs and supporting both node.js and browser by hand (instead of umd). This is bad bacause it exports some global variables like _self in browser and you won't be able to use it in the native esm way because imports order is not guaranteed. See vite#1438 since vite depends on native esm in development mode. This solution is also wrong.

As a result, your package.json should look like this:

{
    // node's require and import, to override it, see "exports"
    "main": "dist/index.js",
    // https://nodejs.org/api/packages.html#packages_conditional_exports
    "exports": {
        "node": {
            "import": "./src/index.mjs",
            "require": "./dist/index.js"
        },
        "default": "./src/index.browser.mjs"
    },
    // front-end bundler read this (like vite or webpack)
    "module": "src/index.browser.mjs"
}
@mAAdhaTTah mAAdhaTTah added this to the 2.0 milestone Jan 17, 2021
@joshgoebel
Copy link

Is the import here just a small wrapper around the existing require CJS version then? Or ...?

@hyrious
Copy link
Author

hyrious commented Mar 22, 2021

Is the import here just a small wrapper around the existing require CJS version then? Or ...?

Well, you don't have to write the wrapper by yourself. Rollup, esbuild and webpack both handle (most of the) cjs libraries (including react) well.

The advantage of using esm is all about tree-shaking. You can treat preact as an example.

@joshgoebel
Copy link

joshgoebel commented Mar 22, 2021

How would tree shaking help a tiny library like Prism that is already super-small and (I presume) most of the functions present are actually truly required for the highlighting process - hence they can't be shaken...

Well, you don't have to write the wrapper by yourself.

Well, yeah, I know things can be automated... but Prism has 100s of files... so I was asking if the ESM would be a top-level wrapper that just requires something else, or if every one of those 100s of files would need both a CJS and ESM version...

@RunDevelopment
Copy link
Member

How would tree shaking help a tiny library like Prism that is already super-small

It won't. Right now, Prism Core only contains one small function that isn't necessary for the highlighting process. But even that function can't be "shaken off" because Prism Core doesn't export individual functions, it exports a Prism instance so all functions are instance methods. As it is right now, Prism won't benefit from tree shaking at all.

That being said, we do plan to go to ESM with v2.0. This will require significant changes to our build pipeline, so generating a CJS and ESM version of Prism shouldn't be an issue then.

@joshgoebel
Copy link

so generating a CJS and ESM version of Prism shouldn't be an issue then. [emphasis mine]

What does that look like then? Is there then both a ESM and CJS version of every single grammar file? IE, an entire sep CJS vs ESM folder depending on how one wants to use it? We're debating ESM for v11 for Highlight.js right now.

@RunDevelopment
Copy link
Member

IE, an entire sep CJS vs ESM folder depending on how one wants to use it?

That's what I had in mind. Since these builds are all generated for us, it's an easy and convenient way to support both.

That being said, we are still in the early stages with the ESM idea, so nothing is final yet.

We're debating ESM for v11 for Highlight.js right now.

I thought HighlightJS was already using it?

@joshgoebel
Copy link

joshgoebel commented Mar 22, 2021

I thought HighlightJS was already using it?

For the raw source, yes, since v10. But our "cooked" source (web distributable, NPM library, etc) is all CJS. Everything is run thru rollup to build the distributable.

@benmccann
Copy link

It looks like ESM output has been added to Highlight.js now: highlightjs/highlight.js#3007

@joshgoebel
Copy link

joshgoebel commented Apr 20, 2021

I'm not sure we'll stick with conditional exports though, we're going to see how the v11 alpha/betas work... might end up just being two folders and someone has to specify the path they want. And of course for many simple use cases we still recommend our pre-built CommonJS monolith CDN build, which behaves pretty much the same as it always has.

@TheMrZZ
Copy link

TheMrZZ commented Feb 4, 2022

Is ESM still in discussion for Prism?

@RunDevelopment
Copy link
Member

RunDevelopment commented Feb 8, 2022

It's planned for v2, so yes.

@RunDevelopment
Copy link
Member

Implemented in v2.

@shellscape
Copy link

@RunDevelopment has v2 been published anywhere? presently banging my head against v1.x trying to load relative files via require.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants