tomwanzek / d3-ng2-service

A D3 service for use with Angular.
MIT License
205 stars 42 forks source link

upgrade to angular 6 #123

Open nhhockeyplayer opened 6 years ago

nhhockeyplayer commented 6 years ago

is this fit for angular 6?

I don't see any upgrade indicators

would be nice

HarelM commented 6 years ago

I'm using it with angular 6 and rxjs-compat, but it would be nice if this get bumped to angular 6 and rxjs 6.

HarelM commented 5 years ago

Angular 7 has just came out, would be nice to be able not to get the peer dependency warning with it too.

HarelM commented 5 years ago

Also it seems that when using typescript 3.1 this breaks compilation due to d3 shape, I'm not sure how to fix this though... See here: https://github.com/DefinitelyTyped/DefinitelyTyped/issues/29587

adumesny commented 5 years ago

I'm using it with Ang 6 and I don't import rxjs-compat (or see it listed in yarn.lock) as I converted my code over. No issue other than peer dependency warning. I haven't tried D3 5.x yet though

HarelM commented 5 years ago

Angular 6 doesn't use typescript 3.1. I managed to fix it using the following flag in typescript config: "skipLibCheck": true,

saberone commented 5 years ago

Should we still consider this package to be maintained? It hasn't been updated for a while now. And everytime a new version of Angular is released I cross my fingers and hope it keeps working.

adumesny commented 5 years ago

I'm now using it with Angular 7 / Typescript 3.1 - just lib dependency warning issues. Note: if you switch to Typescript 3.1+ make sure to use a later version of @types/d3-shape@1.2.7 (remove entry from yarn.lock file + yarn, or remove node_modules dir and npm install again) or you will get error TS2304: Cannot find name 'CanvasPathMethods'

also see #119 and #118

ZeevKatz commented 5 years ago

@nhhockeyplayer @HarelM @adumesny @saberone @saberone Hi everyone! It seems like this repository not maintained anymore, so I created a new repository (and published it to npm) with support for Angular 6 (and 7). Here is a link to ngx-d3

HarelM commented 5 years ago

@ZeevKatz thanks for the info! I'll try to use it and let you know if it works as expected. As a side note, I'm using angular 7 with the current package and it works but I prefer to use a maintained library :-)

ZeevKatz commented 5 years ago

@HarelM If you are using d3-selection-multi it should not work, and I hope you will not run into any issues with ngx-d3! :)

HarelM commented 5 years ago

@ZeevKatz I've updated to your version. Seems to work well. Thanks!!

adumesny commented 5 years ago

@ZeevKatz doing a quick look at your package.json it looks like you are including ALL of d3 instead of cherry picking a subset like d3-ng2-service does ?

"dependencies": { "@types/d3": "^5.7.1", "d3": "^5.9.1" }

that seems like a big downside. Also should give credit to Tom. Thank you for doing this though.

Other than yarn dependencies warning with Ang7 (and 6 before that) this lib still works fine for the lib I publish internally.

ZeevKatz commented 5 years ago

@adumesny the only thing d3-ng2-service excluded from d3 is d3-fetch (and added d3-selection-multi), you can't say it is cherry picking.

Update: I added a credit to Tom IMO.

adumesny commented 5 years ago

@ZeevKatz sorry I meant to give credit to Tom Wanzek for d3-ng2-service since yours is based on that. You could have just branched his and made the mods for angular 6/7 dependencies, instead of a re-write...

I started using yours to see. I looks like d3-selection-multi is NOT exported in your D3 object unlike d3-ng2 (which exports maybe a subset?). I just looked at the yarn.lock file and see the module missing now (I don't think we use it).

Also I would recommend you name yours D3Service instead of NgxD3Service as it would make going from d3-ng2 to yours easier (just change the import from). Right now I have to change code and change the import re-order in tons of file (lint rule to sort import) because of the name change :(