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

[Webpack] Uncaught TypeError: deepmerge is not a function #97

Closed
perry-mitchell opened this issue Feb 26, 2018 · 20 comments
Closed

[Webpack] Uncaught TypeError: deepmerge is not a function #97

perry-mitchell opened this issue Feb 26, 2018 · 20 comments

Comments

@perry-mitchell
Copy link

perry-mitchell commented Feb 26, 2018

Hi,

I've been using deepmerge in many of my libraries, mostly private but a few of my public ones as well. The most popular library of mine to use it, webdav, was recently updated to use deepmerge 2 - the import of deepmerge broke:

Uncaught TypeError: deepmerge is not a function
    at Object.getDirectoryContents (factory.js:118)
    at eval (main_init.js:86)

This has only occurred for me in the 2.* releases. I use commonjs to import deepmerge. The library is basic Javascript compatible with NodeJS 4 and upwards. I've tested that this breaks in a couple of webpack configurations.

EDIT: I've created a reproduction.

@TehShrike
Copy link
Owner

Yup, sounds like #87.

I believe this is a Webpack bug. As of 2.x, deepmerge exposes two entry points in the package.json: a UMD build via main and a ESM build via module. They are both legit and cool.

I don't use Webpack. I'm still waiting for somebody to make a repository with a small reproduction that we can take to the Webpack devs.

If you can make a repo that demonstrates the issue with recent Webpack, I can help get it looked at by the Webpack team.

@TehShrike
Copy link
Owner

All the repo needs: a file that imports deepmerge in a way that makes the error happen, a Webpack config, and npm run scripts that you can run to show the error happening. webpack build && node build-output.js or whatever.

@perry-mitchell
Copy link
Author

perry-mitchell commented Feb 26, 2018 via email

@TehShrike
Copy link
Owner

There is one difference: 1.5.2 had a browser field in the package.json, and 2.0.0 did not.

@perry-mitchell
Copy link
Author

@TehShrike I've created a reproduction here. Instructions on how to run it are in the readme. I'm using node 8 currently and it's easily reproducible with just the couple of lines in the index.js file:

image

@TehShrike
Copy link
Owner

That's excellent. If you're okay with perry-mitchell/repo-deepmerge-webpack#1 and merge it, I'll make an issue over at Webpack.

@perry-mitchell
Copy link
Author

I'm still not sure why you think it's completely webpack that has faulted here, as webpack hasn't changed but deepmerge has. 2.0 realised a breaking change for all webpack users, so imo the responsibility falls to deepmerge to fix it (if it is at all in line with the project's goals to remain compatible).

Waiting on a potential change from webpack seems unlikely at worst, and a long way off at best.

@TehShrike
Copy link
Owner

TehShrike commented Feb 26, 2018

If you open the dist.js file after building the reproduction repo, you can see the bug on line 10. When you import the module with require, Webpack is returning the exports property of the module, which is incorrect.

Take a look at the ESM deepmerge code here: https://unpkg.com/deepmerge@2.0.1/dist/es.js

What about it is wrong? It exports the function via export default.

I'm not motivated to drop the ESM bundle because of a bug in Webpack.

@TehShrike
Copy link
Owner

I opened an issue with Webpack: webpack/webpack#6584

@perry-mitchell
Copy link
Author

Well to be honest I would still point the finger at how deepmerge is packaged - I use hundreds of other modules with webpack without issue. I know it's not specifically a fault with deepmerge, but the presence of the extra bundled es code makes it incompatible. I would say it's at a great cost, as well, but this is all just my opinion.

That being said - I hope that you find a solution. I'll be watching what happens 😊. Good luck with the webpack issue. I'll have to switch to another merge library in the meantime, as this is a serious blocker for me.

@TehShrike
Copy link
Owner

If you're controlling the code that is importing deepmerge, you can always const merge = require('deepmerge').default and you'll get the function you want.

@oliviertassinari
Copy link

oliviertassinari commented May 28, 2018

Have you considered the option to remove this line?
https://github.com/KyleAMathews/deepmerge/blob/dfdb7239fff13385d44b5257f17a7423b8421678/package.json#L20

@TehShrike
Copy link
Owner

But that would mean that the bad bundlers would win!

@QuentinRoy
Copy link

QuentinRoy commented Aug 25, 2018

@TehShrike I'm sorry but I believe you're the one being wrong. There is virtually no chance your proposition will ever get approved by the Webpack team, since there is virtually no chance this will end up being the behaviour of node's loader. Of course, this is still under discussion (mjs extension debate), but it will most likely look like this. This link also provides some recommandation for library author to support both esm and cjs. You may want to use these.

@TehShrike
Copy link
Owner

Thanks for that link @QuentinRoy, I hadn't read that yet!

If it is accurate, then the forward-facing way to expose ESM and CJS modules to node.js would be for deepmerge to:

  1. change the ESM file to use the mjs file extension
  2. change the two output files to have the same base file name (e.g. deepmerge.js and deepmerge.mjs
  3. change the main entry point to not have an extension (e.g. "main": "./dist/deepmerge"

If this is the best path forward, this seems reasonable.

Note: this will not necessarily work around the Webpack bug! This would only make it more likely that you could import deepmerge from 'deepmerge' in node.js and have everything work as you'd hope.

@QuentinRoy
Copy link

QuentinRoy commented Aug 25, 2018

Webpack uses a different module resolution depending on what extension your source file has.
It isn't well documented and I think it is actually quite new, but if you are using .mjs, it will switch to some "node compatibility" mode and will try to find a .mjs script from package.json's "main" field. In this case, it will ignore the "module" field.
For .js files, it will follow the "module" field by default, but I thought that was only the case if you were using mjs (I haven't used cjs for a while). If it also follows "module" even for source files using cjs, that's indeed a bit weird...
For their defence, one of the arguments in favour of the .mjs extension in node, is that the type of loader to use have to be known before the execution of the module. They might just not be able to make the difference..
I'm not sure using cjs with Webpack is a good idea anymore (Webpack is more and more focused on mjs, e.g. as far as I know tree shaking won't work with cjs), but if one really wants to go this way, it is probably a good idea to disable the "module" resolution.

@TehShrike
Copy link
Owner

If it also follows "module" even for source files using cjs, that's indeed a bit weird

Yeah, that's the crux of the Webpack bug I think. I tried to suggest changing that in this comment:

One potential solution would be: when require is used, prioritize the main entry point over the module entry point.


I didn't know that Webpack was inferring anything from file extensions already, that's interesting.

I'm open to any pull requests that implement the three steps I gave a couple posts up - if nobody else implements that, I'll probably get around to it eventually.

At the very least, I'd like to get that done by the time ESM imports land in node.js unflagged.

@TehShrike
Copy link
Owner

I asked a person who knows better than me, and it sounds like that particular path forward isn't guaranteed to be The Right Way, but seems reasonable for now.

@AppointMike
Copy link

This issue is still happening for me. The exact stack trace is

Uncaught (in promise) TypeError: (0 , _deepmergeTs.deepmerge) is not a function
at LocalizationStore.js:74:1
at createStoreImpl (vanilla.js:30:1)
at createStore (vanilla.js:34:1)
at createImpl (index.js:19:1)
at create (index.js:27:1)
at createStore (LocalizationStore.js:71:1)
at createStore (LocalizationStore.js:205:1)
at Provider (context.js:15:1)
at renderWithHooks (react-dom.development.js:14969:1)
at mountIndeterminateComponent (react-dom.development.js:17731:1)

I didn't change any dependencies and today it just began happening. Any ideas as to why this is?

@douglasg14b
Copy link

Given that this is happening without webpack, with normal TS, that indicates this is not in fact a webpack bug.

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

No branches or pull requests

6 participants