-
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
Having trouble importing deepmerge with Webpack #87
Comments
I would guess that there is different bundling/module loading happening between typescript and jest. I'm not familiar with either of them, but my clue to you is to make sure that they're both either paying attention to the |
I'm not using TypeScript, just JS on the browser and on node. v2.0.2 works on node, but fails with the error on a browser. Reverting to v1.5.2 works on both environments. |
@richardschneider What bundler are you using? What is the error message? |
@TehShrike I'm using See code at https://github.com/richardschneider/ipfs-encryption/blob/master/src/keychain.js#L80 |
I couldn't find any webpack config in that repo, but I'm no Webpack expert anyways. In any case, I can guess at the problem pretty easily - your Webpack is configured to load in the ESM module, and is probably exposing default exports as a There are lots of ways you could fix this - you could If you want your un-built code to work in node.js, I'm guessing you'll want to do one of the two latter options. |
Just to update the issue, I have solved this by importing explicitly the file I want, import merge from '../node_modules/deepmerge/dist/es.js'; I am not particularly happy about this, because if the name or path to the file change, the import will break. It is still the best solution I could find. I have asked a question on StackOverflow to see if anybody had any insights to offer, but no answers thus far. I guess you can close this issue if you have nothing to add. Thanks. |
@GioSensation You still haven't responded to my earlier message - it looks like you have two bundler environments, and they're both resolving modules differently. This doesn't have anything to do with this package using Rollup to bundle, it has to do with your bundlers respecting the A quick search of issues in the Jest repo turns up this: jestjs/jest#2702 It looks like somebody in that thread figured out a workaround for Jest. |
Thanks. I will look into it and wait for the Jest team to integrate that pull request. Cheers! |
Could this be reopenend? |
As long as your bundler respects the The package.json's The package.json's Which one of those do you want your bundler to pick up? That is up to your project's configuration. As far as I know, deepmerge's two entry points are the epitome of bundler-friendliness. Any issues should be the result of a misconfiguration or a bug in the bundler that you're using. I don't know if it's a configuration issue or a bug, but it sounds like some combination of Typescript + Webpack has an issue out of the box. |
@Tarnadas If you edit the file index.d.ts from node_modules/types/deepmerge and change the export from If you want to import as I am not sure if the typings reflect the most up to date version of the library. There is only one method that I use and for that one is working. |
ooooh, is that why people are having difficulty with TypeScript? Where are people getting these types from? Can somebody update them? Does Typescript pull from |
There is a single repository that most common libraries types are there. It's mostly updated and maintainted by the community. https://github.com/DefinitelyTyped/DefinitelyTyped. The issue is that a lot of times they are not up to date with the library and this can cause issues when compiling. In this case the problem is more with how TypeScript import works. TypeScript threats CommonJS/AMD/UMD modules in the same way as ES6 modules. When you do The other way would be This option is not enabled by default, but since @Tarnadas was complaining about compiling issues, I figured that this was enabled for him. I also assumed he was using the types from DefinelyTyped. That's why I suggested changing that file. |
I'm having this issue too but deepmerge is being used by a transitive dependency so I can't change the import/require of the function. Any ideas? |
I'm at a disadvantage here because I'm not familiar with TypeScript's module resolution. I'm also not terribly motivated to spend a day learning about it to fix this issue :-x deepmerge offers a good import experience for both CommonJS consumers (setting I don't see any reason to change that in order to facilitate TypeScript. Besides, as more modules start shipping ES Modules, this problem will keep happening to TypeScript users. That said, it would be nice to know how to fix it. Are the typings on DefinitelyTyped correct? Could they be changed to fix the issue? Should an issue be opened on TypeScript? A quick search turns up at least a couple issues related to CommonJS/ESM interop ({#3337](microsoft/TypeScript#3337), #2719), there may be some clues in there. @pacey, does the library that is consuming deepmerge use the If there is some way to work around this TypeScript issue by fiddling with Webpack/Babel config, it would be good to get it on record here. |
It may also just be an issue with Webpack. webpack/webpack#5756 |
Yeah it's quite possible to just be a Webpack issue @TehShrike. I'll try and get webpack/webpack#5756 resurrected! |
@pacey can you make a bare-bones reproduction in a public repository that is just a single-file app that imports deepmerge and gets bundled with webpack? |
I'm also getting this issue, but not with typescript: #97 |
I opened an issue with Webpack: webpack/webpack#6584 |
FWIW I was able to work around setting a resolve alias in the webpack config
|
Cool - that's probably worth putting into the readme until the Webpack issue is fixed. |
I was also having this problem w/ Jest (also using TypeScript, but using Rollup to bundle my library), and I came up w/ a different solution that might work for other Jest users until jestjs/jest#2704 is merged/released. I created a manual mock that // __mocks__/deepmerge.ts
// (where __mocks__ is a sibling to node_modules)
import deepmerge = require('deepmerge')
export default function merge(...args) {
return deepmerge(...args)
} Then I just use FWIW, @TehShrike I think you're doing everything right on the |
if you're just extending simple objects you can quickly roll your own solution, e.g:
|
'is-mergeable-object' should be a dependency not dev-dep, shouldn't it? |
The output is bundled with rollup before deploying, so |
I have a similar problem where it's looking in all places, just not the node_modules: import merge from 'deepmerge' it gives ERROR in /Users/-----/.ghq/github.com/mesqueeb/VuexEasyFirestore/src/utils/setDefaultValues.js
Module not found: Error: Can't resolve 'deepmerge' in '/Users/-----/.ghq/github.com/mesqueeb/VuexEasyFirestore/src/utils'
@ /Users/-----/.ghq/github.com/mesqueeb/VuexEasyFirestore/src/utils/setDefaultValues.js 1:0-29 11:9-14
@ /Users/-----/.ghq/github.com/mesqueeb/VuexEasyFirestore/src/module/actions.js
@ /Users/-----/.ghq/github.com/mesqueeb/VuexEasyFirestore/src/module/index.js
@ /Users/-----/.ghq/github.com/mesqueeb/VuexEasyFirestore/src/index.js
@ ./src/store/store.js
@ ./src/store/index.js Or should I open a new issue? I too am able to fix the issue by writing import merge from '../../node_modules/deepmerge/dist/es.js' instead. But I don't think this is ideal. I'm not sure why this is happening only to deepmerge, not to other npm packages... |
huh, that's super-weird. That does seem like a separate issue, since it just has to do with resolving the package, instead of returning the CJS versus ESM version of the package. |
@TehShrike I also tried editing the Once I am done developing, & rollup my package, i'll let you know if anything has changed. |
@TehShrike I was working on an npm package which had recently added Then it hit me. if I try to To all people trying to test |
I'm having the same problem as @GioSensation where I added
to my jest.config.js which should be importing then the module from that file instead, and now I get a different error
So now it seems to be importing the proper file, but still is throwing an error. |
The DefinitelyTyped typedefs are bad. I uninstalled them and added the following to my project and it works: declare module 'deepmerge' {
function deepmerge<T>(x: Partial<T>, y: Partial<T>, options?: deepmerge.Options): T;
function deepmerge<T1, T2>(x: Partial<T1>, y: Partial<T2>, options?: deepmerge.Options): T1 & T2;
namespace deepmerge {
export interface Options {
arrayMerge?(target: any[], source: any[], options?: Options): any[];
clone?: boolean;
isMergeableObject?(value: object): boolean;
}
export function all<T>(objects: Partial<T>[], options?: Options): T;
}
export default deepmerge;
} If you (deepmerge maintainers) are interested in fixing this within your project, you can include the above in import deepmerge from 'deepmerge';
const opts: deepmerge.Options;
deepmerge(x, y);
deepmerge.all(x, y); |
I'm now open to maintaining a type definition in this project, as long as there is a test that can be added to the npm run scripts, similar to the DefinitelyTyped tests. A PR to this repo would be appreciated! |
In the last delivery (2.2.1) a correction was made on the index.d.ts which is making the Typescript import impossible again… You should keep the
Without this, Typescript users can only do
|
A full-TS option (with better typing as well): |
See #124 |
OK, so this makes sense. But how should we configure Webpack, now, in order not to have the |
There's a workaround in the readme that has worked for some people |
OK, will try. Thank you. |
Defining alias in Webpack is working, but just for information, modules may not be in current project directory (case of workspaces for example). So it would be better not to define absolute path in Webpack configuration. This is how I did in my project:
|
I just added
The project have lot of external dependencies and it would be great to have this fixed regardless possible specific webpack (or other tools) limitations. the workaround for me now is to add in
also to avoid IDE errors I had to add the above also in |
When importing deepmerge with
import * as merge from 'deepmerge';
throwsError: merge is not a function
in the app, while the tests run just fine.When importing with
import merge from
deepmerge;, the app works fine, but the jest tests throw
TypeError: deepmerge_1.default is not a function`.I have temporarily patched the issue in my code with an
if/else
block to reassign the correct function to the variable, but I feel there must be a better way.Maybe jest is misconfigured. Has anyone ever encountered a similar issue or can provide a configuration template that is proven to be working?
This describes a similar issue to what I am experiencing aurelia/skeleton-navigation#606, though I am not using Aurelia, nor Moment. It just seems like the same kind of problem.
Node version: 8.6.0
NPM version: 5.5.1
Webpack version: 3.6.0
Typescript version: 2.5.3
deepmerge version: 2.0.1
tsconfig.json
.babelrc
jest.config.json
The text was updated successfully, but these errors were encountered: