search

TehShrike / deepmerge

A library for deep (recursive) merging of Javascript objects
MIT License
2.75k stars 216 forks source link

Exporting as ES Module Syntax (ESM) #194

Open abdonrd opened 4 years ago

abdonrd commented 4 years ago

Please, export "module" entry point in the package.json as ES Module Syntax (ESM).

I would like to be able to use it directly in the browser, without any build/bundle process.

TehShrike commented 4 years ago

This module used to expose a module entry point, but due to long-lived Webpack bugs, I eventually removed it in #124.

Since so many people use Webpack, which is broken, it wasn't worth the support load to expose both entry points.

However, like you, I find myself often wanting to include modules directly in the browser... maybe the way forward is to publish a CommonJS-only package for Webpack users, and change this package back to exposing both ESM and CJS for everyone else.

daKmoR commented 3 years ago

with the node 12+ now supporting exports in the package.json this should now be possible to cater to both needs 🤗

https://nodejs.org/api/esm.html#esm_conditional_exports

could be something like this

"main": "index.js", // pre node 12
"module": "./index.mjs", // pre node 12 + browser
"exports": {
  "require": "./index.cjs", // node 12+ require
  "default": "./index.mjs", // node 12+ esm and browser
}

thing is it would require a build/copy of the code as esm for the browser needs to be "pure" esm... and require can't load esm code

It is not possible to require() files that have the .mjs extension. Attempting to do so will throw an error. The .mjs extension is reserved for ECMAScript Modules which cannot be loaded via require(). See ECMAScript Modules for more details.

so esm/require would load 2 different files... but as it's a "pure" function that should not be an issue...

if you think that makes sense then I can look into a Pull Request

TehShrike commented 3 years ago

I'm open to adding the exports object, but I'm not sure that would solve the Webpack issue – Webpack would still choke by trying to load the module entry point when someone does require('deepmerge')

RebeccaStevens commented 3 years ago

Only use "exports" and "main". Don't use "module" - Node.js ignores the top-level "module" field, without the "module" field webpack won't have any issues.

"exports": {
  "require": "./index.cjs",
  "default": "./index.mjs"
},
"main": "./index.cjs",
TehShrike commented 3 years ago

But then Rollup and native browser consumers wouldn't be able to consume the ESM module (which is the whole point of this issue per the original post)

RebeccaStevens commented 3 years ago

Hmm, I thought rollup and the likes would be supporting "exports" but after looking it it seems they're not. Here's the original issue on the matter: https://github.com/rollup/plugins/issues/208

Well it would still be good for this package to support "exports" for when it becomes more supported by bundlers. I guess once webpack sorts itself out, the "module" field can be added.

andyburke commented 3 years ago

Hi. Just wanted to weigh in that I'm hitting this issue now and I think it's completely reasonable to split off a cjs version of this that webpack users can use until there's enough pressure on webpack to fix it.

In the meantime, is there any workaround for this?

jorenbroekema commented 1 year ago

I think we're far enough along now with exports (package entrypoints) that all modern tools, bundlers, typescript, etc. etc. support it. So can we do "main" + "exports" field now to support CJS + ESM?

M-a-c-Carter commented 1 year ago

@TehShrike, @jorenbroekema speaks the truth. people who rely on this package dont really want to switch (https://github.com/btroncone/ngrx-store-localstorage/issues/229) and web-pack 5 has been out for a while, we should get esm packages, there are two tickets open about it, this one and https://github.com/TehShrike/deepmerge/issues/250

jorenbroekema commented 1 year ago

https://www.npmjs.com/package/@bundled-es-modules/deepmerge feel free to use ESM bundled fork of this project.

I felt I had no choice but to do this, since the author seems reluctant to add ESM-first support, and deepmerge-ts is almost 4 times as large as original deepmerge.. Of course, you can also commit yourself to a compile-step like @rollup/plugin-commonjs or webpack/esbuild equivalent but for projects that are otherwise compile-step-free, that's not a great option either.

I will deprecate my fork as soon as the author of this package publishes an ESM version.