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

import of deep-diff to Typescript broken with 0.3.5 and 0.3.6 #97

Closed
corvinrok opened this issue Apr 25, 2017 · 18 comments
Closed

import of deep-diff to Typescript broken with 0.3.5 and 0.3.6 #97

corvinrok opened this issue Apr 25, 2017 · 18 comments

Comments

@corvinrok
Copy link

When the releases of the past 2 days, my current code is now broken. I believe this has to do with the imports of the package. This would be fine, except for two things:

  1. The version number updates should be compliant with SEMVER standards and issue a breaking change (IE. 0.4.0, not 0.3.x).
  2. The documentation (README etc) has no explanation of the import requirements of the package that are not ancient es versions.

On previous versions of deep-diff up until 0.3.4, my working code imported this package as:

import { diff } from 'deep-diff';
.. .. ..
.. .. ..
let isDifferent = diff(lh, rh);

This worked excellently. With the release of 0.3.5 and 0.3.6, the compiler now chokes on this import by stating flatly:
TypeError: deep_diff_1.diff is not a function

Punting to a catch-all import like this:

import * as dDiff from 'deep-diff';

also does no good.

I'm losing my mind trying to understand the change and how it can be accommodated and used in Typescript. I tried reviewing the '@types/deep-diff' was no more helpful as it seems not to be current.

What export changes were made to deep-diff, and how should Typescript users now adjust to import this package? (again, all of this could be resolved instantly if there was a useful README.)...

@nassirian
Copy link

nassirian commented Apr 25, 2017

i have a same issue , i downgrade it to 0.3.4 and its worked now.

@flitbit
Copy link
Collaborator

flitbit commented Apr 25, 2017

There was no intentional change in exports — I cannot know, let alone test, all of the scenarios in which people use this library... it was written for nodejs and happily works in most browsers. That being said, now that you've reported an issue with Typescript I'll investigate why you may be encountering issues.

Maybe we can get @thiamsantos to weigh in or take a look, since #92 Change module system of the main file was probably resolved and PR accepted by @flitbit in haste.

I can appreciate the opinion on SEMVER, although semver has not risen to the level of "standard", I'd call it a best practice and I try to adhere; again, unintentional.

At your earliest convenience, please reference the version that previously worked for you, as suggested by @nassirian. If you aren't using a package system that enables such then you can reference the pre-built version that was kindly provided.

In regards to the usefulness of the README, please submit a PR that includes guidelines for usage with Typescript, I'm sure there are others that would appreciate your contribution. I don't use Typescript at all so I probably won't be the most efficient resource in figuring it out.

If I were to start looking though, I'd probably do something like this:

  1. Set up a test to see if importing index.es.js works
  2. If not, import index.js directly and step through lines 2 thru 4 — I'd love to know which path is taken when you run it. The nested ternary that figures out the module system/environment is the essence of what changed. Previously, I had devised my own from bits of code here and there, before there were good patterns in general use.
  3. Submit a PR or report back.

@thiamsantos
Copy link
Contributor

I also don't use Typescript. Regardless that, is important to know how works the module resolution of Typescript. @nassirian @kgentes do you have any idea?

What I know is that two kinds of the module are exposed. Module bundlers that support ES2015 reads the properties pkg.module or pkg.jsnext:main and import the file specified there. And others systems read the property pkg.main and import the file specified there. In other words, who supports es modules import index.es.js and all th rest import index.js.

If Typescript is importing index.es.js the issue can be related on how the methods are defined, because instead of exporting an object with the methods, the es2015 module is actually exporting an object with the default property that is the object that holds the methods of deep-diff.

Given this, I think the approach below should work:

import {default as dDiff} from 'deep-diff'

let isDifferent = dDiff.diff(lh, rh);

@nippur72
Copy link

I use TypeScript and have the issue reported here, anyway I found that the problem is not TypeScript but the Webpack (v2.4.1) module bundler.

As @thiamsantos said, webpack is picking index.es.js instead of index.js and it's generating a wrong default export:

// ... 
__webpack_exports__["default"] = (accumulateDiff);
// ... 

by manually erasing jsnext:main from deep-diff/package.json all works as before.

I wonder if this is related to this webpack issue.

@greysonp
Copy link

I also experienced this issue with plain javascript using Webpack.

var diff = require('deep-diff').diff

This fails to find diff.
Downgrading to 0.3.4 fixes the issue for me.

@corvinrok
Copy link
Author

To answer your question @flitbit , as I said in my initial post, this problem has begun with "the release of 0.3.5 and 0.3.6". My code works fine with version 0.3.4. Sounds like you have a lot of info on this already.

@thiamsantos
Copy link
Contributor

I am trying to solve this issue by translating the methods definition for named exports. With that I was able to solve all cases pointed here, but when you you this module in a CommonJS environment, the following code will throw an error.

var dd = require('deep-diff')
// ...
dd(lhs, rhs)

Will be needed to specify the method.

var dd = require('deep-diff')
// ...
dd.dif(lhs, rhs)
// or
dd.default(lhs, rhs)

But it will work like a charm in ES2015/Typescript code.

import {default as diff} from 'deep-diff'; // explicit 'default'
// is equal to:
import diff from 'deep-diff'; // implicit 'default'
// is equal to:
var a = require('deep-diff').default;

@thiamsantos
Copy link
Contributor

If chosen use completely the ES modules we could also add an explanation on README explaining what to do if you are runnning diff on a CommonJS or browser environment. Redux-thunk is a good example of what can be done. What's your opinion @flitbit? If you like I can send a PR doing that.

@gregbown
Copy link

gregbown commented May 1, 2017

Using deep-diff 0.3.7, TypeScript 2.3.2 with Webpack 2.4.1 the only workable solution I have found is:

import { default as DD } from 'deep-diff';
// usage DD[method]()
DD.observableDiff(this._diff, latest, (d) => {

});

Notes:
Also have "@types/deep-diff": "^0.0.30", in my project package.json
Editing the deep-diff package.json jsnext:main had no effect in resolving issue.

@damianobarbati
Copy link

Any solution here?

import { diff } from 'deep-diff' is broken, no diff found

@thiamsantos
Copy link
Contributor

While there is still no definitive solution, you could do:

import {default as dDiff} from 'deep-diff'

let isDifferent = dDiff.diff(lh, rh);

@damianobarbati
Copy link

Kinda hackish, isn't it? :)

I hoped for true es6 import like
import { deepDiff} from 'deep-diff'
or
import deepDiff from 'deep-diff'

@bogacg
Copy link

bogacg commented Nov 3, 2017

with this setup I didn't have any problem in my node.js test:

ts-node v3.3.0
node v8.9.0
typescript v2.5.3
deep-diff: 0.3.8

just did a simple import * as deep from 'deep-diff' and used it.

vergenzt added a commit to vergenzt/redux-deep-diff that referenced this issue Jan 9, 2018
@bostondevin
Copy link

Yeah - the only way I could get this working after hours with Angular 5 / CLI is downgrading it to 0.3.4 which is disappointing, but back to the races

@flitbit
Copy link
Collaborator

flitbit commented Feb 26, 2018

Please reconfirm with v1.0.0-pre.x...

Install with npm:

npm install deep-diff@next

Many styled imports, diff, DeepDiff, are interchangeable and refer to the same function, the API functions are properties of the default export:

Plain

const diff = require('deep-diff');
// const DeepDiff = require('deep-diff');
import diff from 'deep-diff';
// import DeepDiff from 'deep-diff';

Destructure(ish)

const { diff } = require('deep-diff');
// const DeepDiff = require('deep-diff');
import { diff } from 'deep-diff';
// import { DeepDiff } from 'deep-diff';

@flitbit
Copy link
Collaborator

flitbit commented Feb 26, 2018

Got rid of the ES6 weirdness, it was more trouble that it was worth.

@jcroll
Copy link

jcroll commented Feb 26, 2018

I had some problems importing diff in typescript @flitbit (e.g.):

import { diff } from 'deep-diff';

However I was able to get

import { DeepDiff } from 'deep-diff';

working and it appears so far working fine. Looking good @flitbit 👍

Edit: It should be noted I am using the deep diff library in a typescript bundled library being compiled into a FESM with rollup.js

@nippur72
Copy link

nippur72 commented Aug 5, 2018

this issue is fixed in v1, it can be closed

@flitbit flitbit closed this as completed Aug 10, 2018
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

10 participants