marked icon indicating copy to clipboard operation
marked copied to clipboard

Serving a default export defeats the purpouse of offering both CommonJS and esm

Open KilianKilmister opened this issue 5 years ago • 9 comments

Reasoning

Default esm exports can be useful for their simplicity. Your module exports only a single function or a simple object? A default export can save the enduser a few characters.

// with default export
import whateverNameIWant from 'some-module'
// vs named export
import { someObject as whateverNameIWant } from 'some-module'

But as soon as a module wants to serve multiple things, esm really starts to shine, as the enduser can export exactly what he needs and avoid deconstruction-hell and keep their code considerably more readable and maintainable.

// Need only a few things out of a large library?
import {this, that as someName} from 'large-library'
// Need a large portion of it or simply want to keep it simple?
// use a namespace import:
import * as largeLibrary from 'large-library'

Additionally, TypeScript-Language-Features can interpret esm-modules and will automatically map out a module and a modern editor will use that and not only provide suggestions and autocompletion, but will ship them together with in-file doc-comments and automatic typings. Esm can also be used by a typescript-compiler to generate .d.ts files that will mirror the actual project instead of how manual type definitions can be arbitrarily structured, much to the pain of the enduser. (I can provide the necessary configurations if needed)

Solution

In my opinion it would probably be best to transition the project to natively use esm and use a transpiler to create the CommonJS (the exact opposite of how it is right now). I've done it multiple times before and it is rather painless to do (made even easier by the project using modern ES), adapting to developing using esm isn't very hard and highly rewarding. It is also backwards-compatible so users don't have to change any of their code.

There is an issue for current esm users, tho as offering both default and named exports is considered bad practice (although permitted by the spec). But there is not much way around it so it would probably be best to have an overlap period with the default entering deprecation.

User experience using an esm version of marked

For simple usage one would just have to import the marked() function. And as it would be used and act the exact same way as always.

But for usecases where some heavier modification is needed, a user can easily import and extend things like the Parser class directly.

Concluding...

If this proposal is accepted, i'll gladly assist in the transition. I'm already relatively familiar with the source-code as my usecase requires some heavy modification.

KilianKilmister avatar May 31 '20 08:05 KilianKilmister

We were looking into moving to ESM a while ago but our benchmarks showed cjs was about 10% faster then ESM in Node 12.

Also moving from default to named exports would be a breaking change and I don't think it would be worth it since marked is such a small library the savings from tree shaking would be minuscule.

UziTech avatar Jun 01 '20 04:06 UziTech

Now there has to be something missing or some mistake in this benchmark. ESM modules should theoretically be benefiting a lot from tree-shaking and code-splitting because of their more predictable nature. ESM has been progressing at a high speed, so maybe it was different at the time of the test. but that dipends on when that was of cours.

My point about the default import is less chritical. for my usecases i often have to aggressively disect modules and this is made much easier when it uses named exports. As i'm looking at working on the source directly for this issue #1695, this isn't as important

KilianKilmister avatar Jun 01 '20 05:06 KilianKilmister

From experience. Starting with ESM and generating the CJS and IIFE builds is the easiest/cleanest conversion. ESM with named exports is inherently tree-shakeable so any unused code should be automatically removed when consumers bundle it.

Even if Node is slightly slower, it's not representative of browsers where ESM really shines (especially w/ tree-shaking). I have probably shipped more Node + ESM packages than any other Dev. Feel free to ping me if you'd like help and/or guidance on working w/ ESM.

evanplaice avatar Jul 04 '20 02:07 evanplaice

@evanplaice we do provide an ESM bundle for browsers in lib/marked.esm.js.

It doesn't really make sense to tree shake a library like marked since it is so small and only provides one function that uses all of the code.

UziTech avatar Jul 04 '20 02:07 UziTech

I mean convert the base source to ESM

ESM is easier to optimize. I design even the smallest libs for this.

By adding JSDoc descriptions and types, Intellisense and inline documentation in VSCode will work. With a simple build step, TS typings (ie *.d.ts) can be generated for Typescript compatibility.

CJS, IIFE, and Minified builds are trivial to setup.

I'm only offering to help. Otherwise, the invitation is open.

If you want to see samples of libs that follow this approach check out https://github.com/vanillaes/vanillaes

evanplaice avatar Jul 04 '20 02:07 evanplaice

@evanplaice we are always open to PRs. If you can show a significant improvement in speed or size we will definitely move to ESM. I have no problem with ESM other than my tests have shown that CJS is currently faster than ESM in node LTS v12.x

UziTech avatar Jul 04 '20 04:07 UziTech

@UziTech is there some documentation on the benchmarking and results to use as a reference for future benchmarking?

KilianKilmister avatar Jul 09 '20 09:07 KilianKilmister

@KilianKilmister if you clone marked you can run the benchmarks by running npm run bench the file that runs is test/bench.js

UziTech avatar Jul 09 '20 14:07 UziTech

@UziTech yeah, i found it, thanks.

KilianKilmister avatar Jul 09 '20 17:07 KilianKilmister

This was fixed in v4

UziTech avatar Nov 21 '22 06:11 UziTech