flitbit / diff

Javascript utility for calculating deep difference, capturing changes, and applying changes across objects; for nodejs and the browser.
MIT License
2.99k stars 213 forks source link

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

Closed corvinrok closed 6 years ago

corvinrok commented 7 years ago

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 commented 7 years ago

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

flitbit commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

Any solution here?

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

thiamsantos commented 7 years ago

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

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

let isDifferent = dDiff.diff(lh, rh);
damianobarbati commented 7 years ago

Kinda hackish, isn't it? :)

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

bogacg commented 6 years ago

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.

bostondevin commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

jcroll commented 6 years ago

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 commented 6 years ago

this issue is fixed in v1, it can be closed