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

chore: Update to latest maplibre-gl-js-amplify for maplibre v2 support #2263

Merged
merged 3 commits into from Jul 7, 2022

Conversation

slaymance
Copy link
Contributor

Description of changes

This pull request updates maplibre-gl-js-amplify to version 2.0.0, which includes support for maplibre-gl v2.

This change will negate the need for a separate @aws-amplify/ui-react@studio tag.

Issue #, if available

N/A

Description of how you validated changes

Upgraded packages and ensured all tests pass with no code updates.

Checklist

  • PR description included
  • yarn test passes
  • No side effects or sideEffects field updated

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@slaymance slaymance added dependencies Pull request that updates dependency file React An issue or a feature-request for React platform Geo labels Jul 7, 2022
@slaymance slaymance requested a review from thaddmt July 7, 2022 19:05
@slaymance slaymance requested a review from a team as a code owner July 7, 2022 19:05
@slaymance slaymance self-assigned this Jul 7, 2022
@changeset-bot
Copy link

changeset-bot bot commented Jul 7, 2022

🦋 Changeset detected

Latest commit: 6c7d6d6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@aws-amplify/ui-react Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@@ -134,7 +134,7 @@
"name": "Geo",
"path": "dist/esm/index.js",
"import": "{ MapView, LocationSearch }",
"limit": "300 kB"
"limit": "330 kB"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The size increase is due to the addition of the Geofence feature in the upgraded maplibre-gl-js-amplify package.

@slaymance slaymance temporarily deployed to ci July 7, 2022 19:49 Inactive
@slaymance slaymance temporarily deployed to ci July 7, 2022 19:49 Inactive
@slaymance slaymance temporarily deployed to ci July 7, 2022 19:49 Inactive
@slaymance slaymance temporarily deployed to ci July 7, 2022 19:49 Inactive
@@ -134,7 +134,7 @@
"name": "Geo",
"path": "dist/esm/index.js",
"import": "{ MapView, LocationSearch }",
"limit": "300 kB"
"limit": "330 kB"
Copy link
Contributor

@reesscot reesscot Jul 7, 2022

Choose a reason for hiding this comment

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

I'm not sure we're OK with bumping this limit given this is a jump from 280kB to 330kB, or about an 11.5% jump for an app bundle using MapView and LocationSearch

Copy link
Contributor

Choose a reason for hiding this comment

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

How big is the bundle now? Exactly 330k?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's now:
321.52 kBwith all dependencies, minified and gzipped

Copy link
Contributor

@reesscot reesscot left a comment

Choose a reason for hiding this comment

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

Discussed offline that this bundle size bump for Geo UI component usage is unavoidable with the new GeoFence feature added to maplibre-gl-js-amplify. Separately verified that the extra weight will only be in applications using Geo UI components, and that customers using Authenticator can still treeshake out the Geo dependencies:

Before this PR:
Authenticator
Size: 86.88 kBwith all dependencies, minified and gzipped

After this PR:
Authenticator
Size: 86.88 kBwith all dependencies, minified and gzipped

As seen above, there's no bundle size change for Authenticator users.

Copy link
Contributor

@zchenwei zchenwei left a comment

Choose a reason for hiding this comment

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

LGTM

@slaymance slaymance merged commit 14d35db into main Jul 7, 2022
@slaymance slaymance deleted the maplibre-bump branch July 7, 2022 21:27
@github-actions github-actions bot mentioned this pull request Jul 7, 2022
zchenwei pushed a commit that referenced this pull request Jul 15, 2022
#2263)

* Upgrade maplibre to v2

* Minor version bump changeset for ui-react

* Increase Geo size limit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull request that updates dependency file Geo React An issue or a feature-request for React platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants