-
Notifications
You must be signed in to change notification settings - Fork 217
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
Help using deepmerge as module #137
Comments
What version of deepmerge? (run What bundler are you using? (I'm going to guess Webpack) You'll need deepmerge@3 to get the version that doesn't trigger the Webpack bug. |
deepmerge@3.1.0
|
What's running your unit tests? Everything should work with deepmerge@3.1.0 is a UMD package, so whatever bundler thingy is turning that into an object with a In order for anyone to look into your issue more deeply, you'll probably need to create a repository that demonstrates the issue that people can poke at. |
Sigh, you're right about needing to create a testcase repo. But it's now working for me as |
aah, yeah, I don't know anything about the semantics of Babel's |
@TehShrike I was very surprised to find that this package does not expose a proper ES module. I would expect any well-used and well-maintained modern JS package to transpile to multiple targets. Do you need help and/or a PR? |
@isaachinman this package used to expose an ES entry point, but it was removed in #124 because of a bug in Webpack that caused issues for lots of people. |
It's not a bug, you need a
But, that's exactly right. You cannot Moreover, you might be confused by Babel 5's compatibility hack and conflating that with Webpack. The maintainer of Webpack, @sokra, basically gave the same response a year ago. I am happy to help if you're interested. Just know that maintainers of all JS packages have the same problem, and there are definitely solutions available. Ultimately this package is 64 LOC and should be very simple to export for both ES and CJS. |
The ESM entry point was rolled back because of this specific bug in Webpack, not because of any Babel behavior. Webpack incorrectly uses the Lots of Webpack users hit the issue when one of their dependencies (that they didn't control) would be a CommonJS library containing a |
That very well may be, but it's absolutely no reason to exclude an ES export from any library. You don't seem willing to take that into account, and that's fine! It's a decision for a maintainer to make. I have already found another solution for my own use case. For those dead set on using this package with Webpack and ES modules, you can do: import { default as deepmerge } from 'deepmerge' |
Did you read #87 or #97? If deepmerge publishes a Trying to configure the exports of this library so that the CommonJS and ESM exports would be interpreted consistently by different bundlers would, at the very least, require a breaking change to the CommonJS exports, and I don't feel any motivation to do that. You're in danger of coming off as a bit patronizing here. I'm pretty familiar with the module ecosystem, and you don't seem to have grasped the different types of bundler behaviors that resulted in those long issue threads in this project that I linked to above. Other than it feeling correct and nice to expose an ESM entry point that doesn't have those few lines of UMD cruft (my motivation for exposing the ESM entry point in the first place), there's no huge benefit to implementing ESM in this library, since there's only small benefit to be gained from tree-shaking out the |
Enough said! |
Same problem. Used |
@verheyenkoen what bundler? what version of deepmerge? |
@TehShrike v3.2.0. The current version on npmjs.org (loaded with yarn but should be unrelated) |
What bundler? |
No bundler. Node.js |
uh... node.js doesn't support ESM natively yet, does it? That comes down to what magic are you using to get ESM to work (my first guesses are If you're on node.js, just use |
I believe it's an experimental feature still but I use an esm library. |
Since I think about 8.5 node has supported modules experimentally behind the flag |
node's built-in esm support is only relevant to packages that export an esm module, and even then I think only ones that use the I suppose it makes sense to update the readme saying that you should only use |
Inspired by the recent discussion in #137
@TehShrike no that doesn't really make sense. If you're using the experimental module system in a file you don't have a choice to fall back to require. It's one or the other. We should probably at least document how to use it from experimental-modules seeing as it already works perfectly. Or at least document that our strange import syntax doesn't apply to node experimental modules. |
That sounds like a node.js problem, not a deepmerge problem – how are you supposed to use CJS modules from inside a node ESM module? I don't know the answer. Bear in mind that @verheyenkoen is talking about something completely different, since he's using the |
@TehShrike No no, you can import CJS modules, just not using require, using import export syntax... and it's not a node.js problem, node.js actually has a completely reasonable api for importing our module. Literally |
Ran into this same issue while running Jest on my modules. |
I'm using webpack in a vue.js project. I'm doing
import * as deepmerge from 'deepmerge'
as recommended in the README, but then I can't actually use it: in my unit tests I getTypeError: deepmerge is not a function
when I calldeepmerge(foo, bar)
ordeepmerge.deepmerge(foo, bar)
. If I printJSON.stringify(deepmerge)
I get this:I've tried
deepmerge(foo, bar)
,deepmerge.deepmerge(foo, bar)
and any other combo I can think of but can't make it work. Any hints?The text was updated successfully, but these errors were encountered: