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

Angular 6 Support #1384

Closed
iget-master opened this issue Apr 24, 2018 · 24 comments
Closed

Angular 6 Support #1384

iget-master opened this issue Apr 24, 2018 · 24 comments

Comments

@iget-master
Copy link

iget-master commented Apr 24, 2018

Issue description

Currently this package requires @angular/common@^4.0.0 || ^5.0.0 and @angular/core@^4.0.0 || ^5.0.0. I think that the changes are not so relevant, so probably the migration to support angular 6 is easy.

The main issue that I've found is about the RxJS change to v6.0.0, where some modules changes the path, so we need to update it.

It also might need the release of a 2.0.0 version, since there are many breaking changes on package dependencies

@jimmykane
Copy link

Here for the comments

@agil-NUBBA
Copy link

agil-NUBBA commented May 4, 2018

Hi,

Just tried to run my project, these are the related errors:

node_modules/@agm/core/services/google-maps-api-wrapper.d.ts(2,10): error TS2305: Module '"my-project/node_modules/rxjs/Observable"' has no exported member 'Observable'.
node_modules/@agm/core/services/managers/circle-manager.d.ts(2,10): error TS2305: Module '"my-project/node_modules/rxjs/Observable"' has no exported member 'Observable'.
node_modules/@agm/core/services/managers/data-layer-manager.d.ts(2,10): error TS2305: Module '"my-project/node_modules/rxjs/Observable"' has no exported member 'Observable'.
node_modules/@agm/core/services/managers/info-window-manager.d.ts(1,10): error TS2305: Module '"my-project/node_modules/rxjs/Observable"' has no exported member 'Observable'.
node_modules/@agm/core/services/managers/kml-layer-manager.d.ts(2,10): error TS2305: Module '"my-project/node_modules/rxjs/Observable"' has no exported member 'Observable'.
node_modules/@agm/core/services/managers/marker-manager.d.ts(2,10): error TS2305: Module '"my-project/node_modules/rxjs/Observable"' has no exported member 'Observable'.
node_modules/@agm/core/services/managers/polygon-manager.d.ts(2,10): error TS2305: Module '"my-project/node_modules/rxjs/Observable"' has no exported member 'Observable'.
node_modules/@agm/core/services/managers/polyline-manager.d.ts(2,10): error TS2305: Module '"my-project/node_modules/rxjs/Observable"' has no exported member 'Observable'.
"@agm/core": "^1.0.0-beta.2",
"@angular/*": "6.0.0",
"rxjs": "^6.1.0",
"typescript": "2.7.2"

@jimmykane
Copy link

I solved my issues via installing
npm install rxjs@6 rxjs-compat@6 --save

as described https://github.com/ReactiveX/rxjs/blob/master/MIGRATION.md

The agm maps breaks with RXJS 6 and it needs the compat module for backwards compatability

@ukon1990
Copy link

ukon1990 commented May 4, 2018

I still have an issue with "snazzy-info-window" after the Angular 6 upgrade.
core.js:1601 ERROR Error: Uncaught (in promise): TypeError: elems[0] is not a constructor TypeError: elems[0] is not a constructor at snazzy-info-window.js:130 at ZoneDelegate.push../node_modules/zone.js/dist/zone.js.ZoneDelegate.invoke (zone.js:388) at Object.onInvoke (core.js:4071) at ZoneDelegate.push../node_modules/zone.js/dist/zone.js.ZoneDelegate.invoke (zone.js:387) at Zone.push../node_modules/zone.js/dist/zone.js.Zone.run (zone.js:138) at zone.js:872 at ZoneDelegate.push../node_modules/zone.js/dist/zone.js.ZoneDelegate.invokeTask (zone.js:421) at Object.onInvokeTask (core.js:4062) at ZoneDelegate.push../node_modules/zone.js/dist/zone.js.ZoneDelegate.invokeTask (zone.js:420) at Zone.push../node_modules/zone.js/dist/zone.js.Zone.runTask (zone.js:188) at snazzy-info-window.js:130 at ZoneDelegate.push../node_modules/zone.js/dist/zone.js.ZoneDelegate.invoke (zone.js:388) at Object.onInvoke (core.js:4071) at ZoneDelegate.push../node_modules/zone.js/dist/zone.js.ZoneDelegate.invoke (zone.js:387) at Zone.push../node_modules/zone.js/dist/zone.js.Zone.run (zone.js:138) at zone.js:872 at ZoneDelegate.push../node_modules/zone.js/dist/zone.js.ZoneDelegate.invokeTask (zone.js:421) at Object.onInvokeTask (core.js:4062) at ZoneDelegate.push../node_modules/zone.js/dist/zone.js.ZoneDelegate.invokeTask (zone.js:420) at Zone.push../node_modules/zone.js/dist/zone.js.Zone.runTask (zone.js:188) at resolvePromise (zone.js:814) at resolvePromise (zone.js:771) at zone.js:873 at ZoneDelegate.push../node_modules/zone.js/dist/zone.js.ZoneDelegate.invokeTask (zone.js:421) at Object.onInvokeTask (core.js:4062) at ZoneDelegate.push../node_modules/zone.js/dist/zone.js.ZoneDelegate.invokeTask (zone.js:420) at Zone.push../node_modules/zone.js/dist/zone.js.Zone.runTask (zone.js:188) at drainMicroTaskQueue (zone.js:595)

And I also get this warning message:
client?719c:153 ./node_modules/@agm/snazzy-info-window/directives/snazzy-info-window.js System.import() is deprecated and will be removed soon. Use import() instead. For more info visit https://webpack.js.org/guides/code-splitting/

@iget-master
Copy link
Author

Be careful with the usage of rxjs-compat package, since it's just an workaround and will add a lot of overhead to your project. Now it's necessary due many packages like this that didn't upgraded to RxJS 6 yet, but remove it as soon as possible.

@Epenance did a nice job updating the package to rxjs@6.1.0, but I think that before making it fully Angular6 compatible, need to check for any breaking changes.

@john-hi
Copy link

john-hi commented May 7, 2018

@ukon1990 the angular 6 switched to use webpack 4, which changed behavior while dynamic importing commonjs modules. I don't know why but snazzy-info-window directive is using dynamic import snazzy-info-window library, that's why there are one warning and broken snazzy-info-window directive. To fix it https://github.com/SebastianM/angular-google-maps/blob/1a74b3a9670cf4dbf442033a17dea290dfadc408/packages/snazzy-info-window/directives/snazzy-info-window.ts#L228
should be changed to this._nativeSnazzyInfoWindow = new elems[0].default(options); but it is not backward compatibility solution (it won't work in angular5/webpack3). I don't know how fix it right.

@Epenance
Copy link

Epenance commented May 7, 2018

I'm pretty sure @SebastianM just needs to bump the packages one major version as Angular 6 breaks a lot of stuff in backwards compat.

@sebholstein
Copy link
Owner

@Epenance yep, that's what I probably have to do, which is a little bit annoying because I have to manage two branches (because too less people will be on v6 any time soon).

@john-hi
Copy link

john-hi commented May 7, 2018

angular-google-maps works fine on v6 with rxjs-compat (except @iget-master remark), but snazzy-info-window directive is really broken right now. Maybe the simplest way is changing dynamic import to static.

@jimmykane
Copy link

jimmykane commented May 7, 2018

@SebastianM you can for now:

  • Upgrade AGM package to RXJS6+RXJSCompat
  • Let us drop the RXJSCompat

Everyone should be happy no?

@peterpeterparker
Copy link

peterpeterparker commented May 7, 2018

@SebastianM about two branches, there is also the ngx-translate way to upgrade, one single branch, last release only compatible with v6, no angular backwards compatibility

not saying that's absolutely the way, just wanted to point out that another major library (in terms of monthly download) choosed such a path 😉

@sebholstein
Copy link
Owner

@jimmykane nope, as rxjs 6 is not compatible with angular 4/5. I think a have a solution...PR is coming

@sebholstein
Copy link
Owner

@peterpeterparker I think it's reasonable to support the latest two version, which are now angular 5/6 and I found a backwards compatible way (that should even work with angular 4 for now but I will flag the next version with support for angular 5/6) - But happy to discuss it further

@peterpeterparker
Copy link

@SebastianM no worries at all, just wanted to bring the info, I'm happy with any way since I already migrated my website to v6 😉

@iget-master
Copy link
Author

IMO, since the migration to Angular 6 is almost pain-less (at least on our projects), I see no reason for maintaining a version for 4 and 5. Just provide dangerous security fixes.

@andregomars
Copy link

@SebastianM really needs it to be upgraded natively

@semoal
Copy link

semoal commented May 8, 2018

Will we have an stable release for Angular 6 support? Working ok with rxjs-compat, but I'll like to remove it asap

@nitinht1988
Copy link

Hey..any update on this?

@manuelsanchezaponte
Copy link

Oh! I just upgrade to Angular 6 and I have the problem on Snazzy-info-window you are discussing:

Error: Uncaught (in promise): TypeError: elems[0] is not a constructor
TypeError: elems[0] is not a constructor
at snazzy-info-window.js:130

Do you have any idea how to fix this?

Thank you!

@LordShiroe
Copy link

@manuelsanchezaponte for now, the @john-hi solution works, change the line 130 to _this._nativeSnazzyInfoWindow = new elems[0].default(options); in the node_modules/@agm/snazzy-info-window/directives/snazzy-info-window.js. This is a temporary solution while the package supports angular 6.

@attilacsanyi
Copy link

attilacsanyi commented May 18, 2018

@LordShiroe are right that seems a quick fix. AGM is the last blocker to upgrade to v6.

@attilacsanyi
Copy link

Thanks @SebastianM appreciate for the quick fix! working like a charm. 🥇

@sebholstein
Copy link
Owner

Whoa you're a fast tester :D glad it works 😅

@peterpeterparker
Copy link

@SebastianM thx a lot 👍

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