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

feat: make setAccessToken(null) obsolete #593

Merged

Conversation

OleksiiZubko
Copy link
Contributor

@OleksiiZubko OleksiiZubko commented Jan 5, 2025

Description

According to comment #25 (comment) , we can drop supporting this.

Related-To #25

Checklist

  • I have tested this on a device/simulator for each compatible OS
  • I formatted JS and TS files with running yarn lint:eslint:fix in the root folder
  • I have run tests via yarn test in the root folder
  • I updated the documentation with running yarn codegen in the root folder
  • I mentioned this change in CHANGELOG.md
  • I added/updated a sample (/packages/examples)

Screenshot OR Video

@OleksiiZubko OleksiiZubko force-pushed the feat/deprecate-access-token-support branch from 8529097 to e8c89bd Compare January 5, 2025 02:03
@KiwiKilian
Copy link
Collaborator

KiwiKilian commented Jan 5, 2025

Thanks for having a look into this!

Is there any benefit in exposing createMapLibreInstance? Preferably one wouldn't have to call a method imperatively from the consumer side at all. Could we call this from the Constructor or something similar, then we wouldn't even need to bring this into the JS context? As we currently recommend to call this as early as possible, I don't see a downside of calling it ourselves.

@OleksiiZubko
Copy link
Contributor Author

OleksiiZubko commented Jan 5, 2025

Yep, makes sense. Let me try

@OleksiiZubko OleksiiZubko force-pushed the feat/deprecate-access-token-support branch from 056e8c4 to d7e367e Compare January 5, 2025 12:31
@OleksiiZubko
Copy link
Contributor Author

@KiwiKilian I've updated it, please let me know if any changes are needed.

@KiwiKilian
Copy link
Collaborator

Awesome, will try this later. I think a deprecation in JS is missing?

@OleksiiZubko OleksiiZubko force-pushed the feat/deprecate-access-token-support branch from d7e367e to f0e581c Compare January 5, 2025 12:46
@OleksiiZubko
Copy link
Contributor Author

you're right, updated. Thanks!

@KiwiKilian
Copy link
Collaborator

I really like this approach, makes usage of the library much easier.

@tyrauber how would you feel about removing it now? This would mean for anyone using this library to remove their setAccessToken(null) line. Anyone using this and setting a proper value shouldn't be using this library anyways. Therefor I'm not sure if a deprecation is helping anyone.

@KiwiKilian
Copy link
Collaborator

KiwiKilian commented Jan 5, 2025

@OleksiiZubko does this work with the latest changes for you? React Native example app on Android crashes on launch for me.

@OleksiiZubko
Copy link
Contributor Author

Yes, I've tested on my real Android device and ios simulator.

@OleksiiZubko
Copy link
Contributor Author

OleksiiZubko commented Jan 5, 2025

I'm testing like this:

yarn example:expo purge && yarn && yarn example:expo android (and yarn example:expo ios)

...

Should I build and run something another?

@OleksiiZubko
Copy link
Contributor Author

@KiwiKilian example:react-native crashes, I'm investigating.

@KiwiKilian
Copy link
Collaborator

Yes was experiencing the same. Expo works, but not RN. Can you check if RN works right now on beta?

@OleksiiZubko
Copy link
Contributor Author

on beta works, no icons in the menu, but I see a map.

@OleksiiZubko OleksiiZubko force-pushed the feat/deprecate-access-token-support branch from 5f020cd to a462027 Compare January 5, 2025 18:06
@OleksiiZubko
Copy link
Contributor Author

OleksiiZubko commented Jan 5, 2025

@KiwiKilian, I've fixed the crash.

I tested in this way on my real device:

git clean -fdx && yarn
yarn example:react-native android
yarn example:expo android

No crash observed and I see the Map.

Please let me know if I should build&test something else.

@KiwiKilian
Copy link
Collaborator

The fix looks great and works fine now on Expo and React Native. I've pushed a migration note. Now lets wait for feedback on removal or deprecation by @tyrauber.

@KiwiKilian KiwiKilian changed the title chore: deprecate access token support feat: make setAccessToken(null) obsolete Jan 5, 2025
@KiwiKilian KiwiKilian requested a review from tyrauber January 5, 2025 20:15
@KiwiKilian
Copy link
Collaborator

This would als resolve #13, right?

Signed-off-by: Oleksii Zubko <[email protected]>
@KiwiKilian KiwiKilian self-requested a review January 5, 2025 20:41
Signed-off-by: Oleksii Zubko <[email protected]>
Copy link
Collaborator

@KiwiKilian KiwiKilian left a comment

Choose a reason for hiding this comment

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

Looking good, if we stay with deprecations I wouldn't require anymore changes. Awesome first time contribution, highly appreciate your work!

@tyrauber
Copy link
Collaborator

tyrauber commented Jan 6, 2025

Yeah, this is fantastic! I'd say merge. Let's get this in 10.0.0. Thanks so much @OleksiiZubko OleksiiZubko

@KiwiKilian KiwiKilian merged commit df44b48 into maplibre:beta Jan 6, 2025
13 checks passed
@KiwiKilian
Copy link
Collaborator

I'm tracking deprecations to be removed in v11 in #580.

github-actions bot pushed a commit that referenced this pull request Jan 6, 2025
# [10.0.0-beta.18](v10.0.0-beta.17...v10.0.0-beta.18) (2025-01-06)

### Features

* make `setAccessToken(null)` obsolete ([#593](#593)) ([df44b48](df44b48))
Copy link

github-actions bot commented Jan 6, 2025

🎉 This PR is included in version 10.0.0-beta.18 🎉

The release is available on:

Your semantic-release bot 📦🚀

@OleksiiZubko OleksiiZubko deleted the feat/deprecate-access-token-support branch January 6, 2025 23:06
github-actions bot pushed a commit that referenced this pull request Jan 14, 2025
# [10.0.0](v9.1.0...v10.0.0) (2025-01-14)

### Bug Fixes

* `VectorSource` `onPress` returning null geometry on Android ([250ee6f](250ee6f)), closes [#538](#538)
* `VectorSource` `onPress` returning null geometry on Android ([a130786](a130786)), closes [#538](#538)
* add @babel/plugin-transform-private-methods for jest ([3a2188f](3a2188f))
* add generic expo config plugin to remove duplicate signature ([#453](#453)) ([2671381](2671381))
* added GeometryCollection to GeoJSONUtils ([#556](#556)) ([e6b7a66](e6b7a66))
* allow MapView and Images to have no children ([#521](#521)) ([1e35bf6](1e35bf6))
* allow resetting contentInset with 0 ([#468](#468)) ([1fe42c6](1fe42c6))
* android example crashing on launch ([#372](#372)) ([aeef5c3](aeef5c3))
* cleanup yarn setup ([#463](#463)) ([d9a4d30](d9a4d30))
* corepack enable  on publish workflow ([2d13f33](2d13f33))
* correct types in MapView ([#268](#268)) ([0ea35c4](0ea35c4))
* disable code signing for release builds ([b3cf088](b3cf088))
* disable library code signing ([22030dd](22030dd))
* empty pbxproj and dwarf-with-dsym plugin config for EAS ([#458](#458)) ([0d54b46](0d54b46))
* expo-app should load library from workspace:. ([016b44a](016b44a))
* export custom header methods ([#552](#552)) ([58abdb0](58abdb0)), closes [#551](#551)
* group dependabot commits by core, dev and example ([#165](#165)) ([b697978](b697978))
* keep [@ts-ignore](https://github.com/ts-ignore) for headingIcon in library [#476](#476) ([#477](#477)) ([ef62454](ef62454))
* make `follow` props on `Camera` deterministic ([#550](#550)) ([e9256e7](e9256e7))
* make MarkerView props with defaults optional ([#460](#460)) ([185cf3e](185cf3e))
* plugin for debug simulator ([#164](#164)) ([06b23d4](06b23d4))
* remove AbortController test mock ([#403](#403)) ([698b558](698b558))
* round compass margins and attribution position to nearest integers [android] ([#294](#294)) ([c89c842](c89c842))
* setMaxAnimationFps on null ([#440](#440)) ([2884256](2884256))
* style expressions ([#466](#466)) ([2202908](2202908))
* trigger release after npm maintenance ([#548](#548)) ([f0fca00](f0fca00))
* types of `getPointInView` and `getCoordinateFromView` ([#601](#601)) ([c7537b5](c7537b5))
* updated Mapbox callstack check for iOS custom headers to check for MapLibre instead ([#461](#461)) ([a6d6216](a6d6216))
* use UIManager exported from react-native ([#511](#511)) ([a4030b5](a4030b5))
* yarn implementation ([#419](#419)) ([39233b1](39233b1))

### Continuous Integration

* add semantic release ([#526](#526)) ([069b6c5](069b6c5))

### Features

* add Expo plugin props ([#589](#589)) ([51fbb00](51fbb00))
* align react and react-native versions for development ([b92abfe](b92abfe))
* allow using google location engine on Android ([#586](#586)) ([92ffdb7](92ffdb7))
* configure packages/examples ([c4510c3](c4510c3))
* drop `MapLibreGL` naming and deprecate `export default` ([#567](#567)) ([aa0c73d](aa0c73d))
* export RegionPayload and MapLibreRNEvent types ([#544](#544)) ([b342f1b](b342f1b))
* extract android UserLocation FPS ([#428](#428)) ([8c0abaa](8c0abaa))
* make `setAccessToken(null)` obsolete ([#593](#593)) ([df44b48](df44b48))
* make Camera pure ([#471](#471)) ([23ecf88](23ecf88))
* MapLibre Android SDK 11.5.0 ([#455](#455)) ([042b759](042b759))
* monorepo configuration ([343e7ac](343e7ac))
* mv example packages/react-native-app ([5c9d3d0](5c9d3d0))
* mv examples, styles, assets, utils and scenes to packages/examples ([13600fe](13600fe))
* packages/expo-app ([c01abd5](c01abd5))
* remove deprecations ([#543](#543)) ([0c41ada](0c41ada))
* remove duplicate of `OfflinePackStatus` type ([#542](#542)) ([9e231b7](9e231b7))
* remove Style component ([#547](#547)) ([9d4c458](9d4c458))
* remove style property enums ([#558](#558)) ([b89a0dd](b89a0dd))
* setup build step ([#504](#504)) ([a017d64](a017d64))
* shared dependencies through packages/examples ([01a9586](01a9586))
* support new arch through interop layer ([#483](#483)) ([951e9cf](951e9cf))
* unify `MapView`s `styleURL` and `styleJSON` to `mapStyle` ([#559](#559)) ([7d22f16](7d22f16))
* update maplibre native version ([#61](#61)) ([25c418a](25c418a))
* upgrade [@Turf](https://github.com/turf) to v7 and remove geo utils ([#478](#478)) ([a45fc55](a45fc55))
* upgrade Android gradle setup ([#539](#539)) ([761ae0d](761ae0d))
* upgrade dependencies ([#535](#535)) ([047f87f](047f87f))
* upgrade MapLibre Native ([#563](#563)) ([d2b7f5d](d2b7f5d))
* upgrade MapLibre Native Android to v11.8.0 ([#597](#597)) ([410d0c3](410d0c3))
* upgrade MapLibre Native iOS to v6.10.0 ([#598](#598)) ([b596c76](b596c76))
* use `withPodfile` instead of `withDangerousMod` in Expo Plugin ([#587](#587)) ([56d02e1](56d02e1))

### BREAKING CHANGES

* remove `styleURL` and `styleJSON` from `MapView`, use `mapStyle` instead
* Removed style property enums
* Remove `Style` component, use `styleJSON` of `MapView` instead
* - Deprecated `UserTrackingModes` is removed in favor of `UserTrackingMode`
- Removed deprecated `setCamera` from `MapView`
- Removed deprecated `byId` methods from `ShapeSource`
- Removed deprecated `children` from `SymbolSource`
- Removed deprecated `assets` key from `Images`
- Removed deprecated event keys
* Replace OfflineProgressStatus with OfflinePackStatus
* Upgrade native packages and migrate components

* ci: move native builds to review

* ci: run release immediate for debugging

* ci: use android working directory for build

* docs: remove RELEASE.md

* chore: remove manual changelog task

* ci: enable release on beta branch

* ci: keep default tagFormat

* ci: setup npm tag fixes

* ci: run review on mr to beta

* ci: run fix tags on beta

* ci: fix name

* ci: clarify workflow_call

* ci: disable debugging

* ci: run fix tags in pr

* ci: setup fix tags to run on beta

* docs: prepare changelog for semantic-release

* ci: remove fix npm tags workflow
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove broken mapbox URL support Decide how to handle the upstream Android divergences
3 participants