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

inspire-tree-dom 3.0 doesn't work with browserify #4

Closed
mreinstein opened this issue Sep 6, 2017 · 7 comments
Closed

inspire-tree-dom 3.0 doesn't work with browserify #4

mreinstein opened this issue Sep 6, 2017 · 7 comments

Comments

@mreinstein
Copy link

prior to the 3.0 release I was able to bundle inspire-tree and inspire-tree-dom into my application via browserify. Now it fails, with this console error:

Uncaught TypeError: Tree argument is not an InspireTree instance.

offending line: https://github.com/helion3/inspire-tree-dom/blob/master/src/dom.js#L18

@viveleroi
Copy link
Contributor

Can you provide an example browserify config? Trying it purely with an example implementation and browserify src.js -o dist.js doesn't reproduce the issue.

src.js being:

const InspireTree = require('inspire-tree');
const InspireTreeDOM = require('inspire-tree-dom');

const tree = new InspireTree({
    data: []
});

const dom = new InspireTreeDOM(tree, {
    target: '.tree'
});

Are you using requirejs or anything as well?

@mreinstein
Copy link
Author

mreinstein commented Sep 7, 2017

@viveleroi here is the minimal repro example:

1 clone https://github.com/mreinstein/it-fail-example
2 npm i
3 npm run start-dev
4 open http://localhost:5000
5 notice the error in the console

@mreinstein
Copy link
Author

λ node -v
v8.4.0
λ npm -v
5.3.0

@viveleroi
Copy link
Contributor

TL;DR: Update to v3.0.1 of both packages and should be good. Verify both are correctly listed in your package-lock.json.

This is of a trail of problems:

  • v2 of inspire-tree-dom went out with inspire-tree v2 listed as a dependency in package.json, as well as a peerDep. I noticed this and fixed it right after v3 went out, but hadn't yet released it.
  • NPM apparently chose the dep, not the peerDep to write in your lockfile. If you look at the package-lock.json file generated, it has the wrong version:
"dependencies": {
    "inspire-tree": {
      "version": "3.0.0",
      "resolved": "https://registry.npmjs.org/inspire-tree/-/inspire-tree-3.0.0.tgz",
      "integrity": "sha512-BJjDveZ00Erw2ZtjrFom0IHFuWrS1We79CCAKZ5hmFEz2BG6CzvDwR/rg3rZ9KlNRVYq18uEO8bBoaH2AWtnbQ=="
    },
    "inspire-tree-dom": {
      "version": "3.0.0",
      "resolved": "https://registry.npmjs.org/inspire-tree-dom/-/inspire-tree-dom-3.0.0.tgz",
      "integrity": "sha512-ibrLpIe47DMin/ATn7WixkKyLbXbYf4QUwaVzIhDsGsWNJVkVDpYnpex+CvPLVDNhDfahxEQJVq4E73hBjRJCg==",
      "requires": {
        "inspire-tree": "2.0.6"
      },
      "dependencies": {
        "inspire-tree": {
          "version": "2.0.6",
          "resolved": "https://registry.npmjs.org/inspire-tree/-/inspire-tree-2.0.6.tgz",
          "integrity": "sha512-EzchngA6S0NrhIIuPVmAA7E+Mj+MvSnuIilnR4fOTo3/qA/8QCLu1ApzGOyLeZ7KycMNB+ofAyVGelSu358WVQ=="
        }
      }
    }

Browserify doesn't know what to do, so it includes both inspire-tree v3 and inspire-tree v2. The bundle.js your repo generated has both...

... and that's why the DOM instanceof test failed, they were different modules.

For what it's worth, Yarn doesn't have this problem. I'm not sure this something we can hold against NPM, but I've been pretty dissatisfied with NPM for some time now and the reason I couldn't reproduce this was because I used yarn.

@mreinstein
Copy link
Author

Thanks for fixing the problem!

but I've been pretty dissatisfied with NPM for some time now and the reason
I couldn't reproduce this was because I used yarn.

I have mixed feeling about npm vs yarn. I get that yarn solves some very real problems in the enterprise space. For example, flattening the dependency tree is nice. On the other hand, it's very frustrating because npm is open source, and the devs @ facebook didn't even try to make fixes in npm. Instead we have a whole other system. now we have 2 fucking module managers that are almost the same except when they're not. :)

@viveleroi
Copy link
Contributor

Having two instead of one is a real problem across the board, so I hear you there.

xkcd

Many of the problems with npm were foundational, and not something the yarn team had the ability to fix with PRs. The rest were ideological. I believe they discussed this in the announcement. The flatter directory is great but the deterministic installs was a vastly more critical feature, npm's shrinkwrap was painful to work with. Yarn is faster, is nicer to use in my opinion, etc. NPM5 is clearly written to address the issues yarn undercut them on (it introduced the package-lock, tries to speed things up, and etc) but npm v5 is unfortunately panned in almost every community I pay attention to.

The list of the breaking/critical bugs left is a bit shocking. They released too early IMO.

I have no affiliation with yarn, but as an open source and commercial developer it's been excellent, I've never looked back.

@mreinstein
Copy link
Author

haha, nice. xkcd for the win.

I def aggree that they released it too early. I think there is probably a sense of desperation in trying to address the people that left for yarn, and bring people back into the fold. The bugs are painful but in my personal stuff I'm sticking with core npm because it's still the majority, and I'm confident that eventually it will stabilize. one can hope anyway. :)

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

2 participants