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

update to angular 10 and @types/googlemaps #1833

Merged
16 commits merged into from Jul 20, 2020
Merged

update to angular 10 and @types/googlemaps #1833

16 commits merged into from Jul 20, 2020

Conversation

ghost
Copy link

@ghost ghost commented Jul 2, 2020

a major change, updates to angular 9 and 10, switch to angular cli building, and switch to @types/googlemaps typings

@ghost ghost requested a review from sebholstein July 2, 2020 19:28
@codecov
Copy link

codecov bot commented Jul 2, 2020

Codecov Report

Merging #1833 into master will increase coverage by 1.45%.
The diff coverage is 48.37%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1833      +/-   ##
==========================================
+ Coverage   47.97%   49.43%   +1.45%     
==========================================
  Files          47       28      -19     
  Lines        2001     1325     -676     
  Branches      178      124      -54     
==========================================
- Hits          960      655     -305     
+ Misses       1037      668     -369     
+ Partials        4        2       -2     
Impacted Files Coverage Δ
packages/core/src/lib/directives/info-window.ts 27.90% <0.00%> (ø)
...re/src/lib/services/managers/data-layer-manager.ts 10.52% <0.00%> (ø)
...e/src/lib/services/managers/info-window-manager.ts 15.15% <0.00%> (ø)
...ore/src/lib/services/managers/kml-layer-manager.ts 25.00% <0.00%> (ø)
...rc/lib/services/maps-api-loader/maps-api-loader.ts 100.00% <ø> (ø)
packages/core/src/lib/utils/browser-globals.ts 60.00% <ø> (ø)
...s/core/src/lib/services/google-maps-api-wrapper.ts 4.76% <4.76%> (ø)
...terer/src/lib/services/managers/cluster-manager.ts 31.94% <27.27%> (ø)
.../core/src/lib/services/managers/polygon-manager.ts 71.73% <37.50%> (ø)
packages/core/src/lib/directives/marker.ts 30.55% <44.44%> (ø)
... and 45 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea471aa...f52b9f9. Read the comment docs.

@loicgasser
Copy link

Wow, thank you for this huge effort.

@Sampath-Lokuge
Copy link

Yes, this is great news. We hope you'll release this soon. Thank you!

@iget-esoares
Copy link

Hope someone merge this soon

@TxusBlack
Copy link

Thank you! We are exciting to use this update!

@ghost ghost marked this pull request as draft July 12, 2020 10:12
@ghost
Copy link
Author

ghost commented Jul 12, 2020

converted to draft because need to fix "Guides" page

@ghost ghost marked this pull request as ready for review July 12, 2020 11:00
@sp90
Copy link

sp90 commented Jul 13, 2020

I don't have extensive knowledge about the project but I can give it a review, especially because I would love for this to go out :D

@ghost
Copy link
Author

ghost commented Jul 13, 2020

thanks @sp90 , but we need a review from @SebastianM

@Foreverdie Foreverdie mentioned this pull request Jul 14, 2020
@sp90
Copy link

sp90 commented Jul 14, 2020

@SebastianM we are awaiting your holyness 👍 🥇

Copy link

@sp90 sp90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have found a few questionable changes, otherwise, it seemed pretty flawless. Could also be missing domain knowledge, but for what its worth here is a review 🚀

README.md Show resolved Hide resolved
package.json Show resolved Hide resolved
packages/core/src/lib/directives/circle.ts Show resolved Hide resolved
packages/core/src/lib/directives/rectangle.ts Show resolved Hide resolved
@ghost ghost assigned sebholstein Jul 14, 2020
Ephraim added 15 commits July 17, 2020 00:54
…h to using angular cli builder-switch to @types/googlemaps-use yarn workspaces for building-fix guides page disappearence-remove some unused files-remove visible attribute from bicycle and transit layer-update target to es2018-rename js-marker-clusterer to markerclustererBreaking: - minimum angular usage is 9.0 - no more [visible] attribute on transit and bicycle layers - JsMarkerClusterer renamed to MarkerClusterer
AgmDataLayer style Input should have the correct type

fixes: #1763
update codebase to conform to tslint and codelyzer rules
@explooosion
Copy link

It looks great!

Does it need to add @type/googlemaps to peerDependencies or dependencies in packages/core/package.json?

@ghost
Copy link
Author

ghost commented Jul 17, 2020

@explooosion @types/googlemaps is already in devDependencies for each of the packages.

@SebastianM please confirm that you were able to get it to build on your local, possibly with a newer yarn version. It doesn't only build successfully on my machine, it also works on netlify and travis

There does need to be an overall effort to sort all dependencies in their right places.

@shadow1349
Copy link

@doom777 thanks for these updates! I'm super excited to update my project.

different packages had wrong or missing data in their package.json
@sebholstein
Copy link
Owner

@doom777 All packages are now published 🎉 . I got it published in an hour or so. The issue was the @types/googlemaps packages. In build:prod mode, I assume that the package gets ignored, so the typescript compiler throws some errors for the types. In non-prod mode, the build went fine.

I got it working by manually install the package before building in prod mode:

 npm i --no-save @types/googlemaps@^3.39.8

After this, I could build/publish the core but the next problem was that I couldn't build the other packages. I had to install @agm/core@3.0.0-beta.0 in all other packages before building.

Maybe you can rm -rf all your node_modules, run npm install again in all packages and then run npm build:prod to see if you can reproduce it.

@ghost ghost merged commit 7982bfb into sebholstein:master Jul 20, 2020
@ghost
Copy link
Author

ghost commented Jul 20, 2020

@SebastianM I am still worried over building. It's NOT supposed to be built with npm, it's supposed to be installed and built with yarn. Then, you don't have any missing dependencies.

I just tested the following in a clean directory

git clone git@github.com:SebastianM/angular-google-maps.git
cd angular-google-maps
yarn
yarn build 

it worked without a hitch, without a need to install additional types or libraries or copy folders into node_modules. Same for yarn build:prod Please confirm you can do the same.

@khayargoli
Copy link

khayargoli commented Jul 21, 2020


Angular CLI: 10.0.3
Node: 12.13.1
OS: win32 x64

Angular: 10.0.4
... animations, common, compiler, compiler-cli, core, forms     
... language-service, platform-browser, platform-browser-dynamic
... router
Ivy Workspace: <error>

Package                           Version
-----------------------------------------------------------     
@angular-devkit/architect         0.1000.3
@angular-devkit/build-angular     0.1000.3
@angular-devkit/build-optimizer   0.1000.3
@angular-devkit/build-webpack     0.1000.3
@angular-devkit/core              10.0.3
@angular-devkit/schematics        10.0.3
@angular/cdk                      10.0.2
@angular/cli                      10.0.3
@angular/material                 10.0.2
@ngtools/webpack                  10.0.3
@schematics/angular               10.0.3
@schematics/update                0.1000.3
rxjs                              6.6.0
typescript                        3.9.7
webpack                           4.43.0

USING: "@agm/core": "^3.0.0-beta.0",

image

Hi guys, Got this error recently. Any fix?

@explooosion
Copy link

Maybe @type/googlemaps should add to dependencies rather than devDependencies.

@Biswas123 npm install @types/googlemaps ?

@khayargoli
Copy link

Maybe @type/googlemaps should add to dependencies rather than devDependencies.

@Biswas123 npm install @types/googlemaps ?

Oh man! thanks! worked.

@ghost
Copy link
Author

ghost commented Jul 21, 2020

@explooosion maybe

@ghost ghost deleted the 3.0.0 branch July 22, 2020 15:18
@michaelz
Copy link

npm ERR! notarget No matching version found for @agm/js-marker-clusterer@^3.0.0-beta.0.

Is there something I need to be aware of ?

@ghost
Copy link
Author

ghost commented Jul 24, 2020

@michaelz yea, it was renamed to @agm/markerclusterer

This pull request was closed.
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

Successfully merging this pull request may close these issues.

None yet

10 participants