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

[notifications][docs] rename and export getExpoPushTokenAsync parameter type #21104

Merged
merged 5 commits into from
Feb 9, 2023

Conversation

Simek
Copy link
Collaborator

@Simek Simek commented Feb 7, 2023

Why

Refs: https://exponent-internal.slack.com/archives/C1QMWQ1P0/p1675723296628999

How

After switching Notifications package to leverage the autogenerated docs one of the methods parameters types was missing. This PR renames and exports the problematic param type to make the documentation complete.

Test Plan

The updated docs data file was tested locally, by running docs app.

Preview

Screenshot 2023-02-07 at 11 45 01

Checklist

@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Feb 7, 2023
@kbrandwijk
Copy link
Contributor

I guess the fact that this type wasn't exported before makes this not a breaking change.

@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Feb 7, 2023
@Simek
Copy link
Collaborator Author

Simek commented Feb 7, 2023

Generally, changing types is not considered by us as a breaking change. In this case, since the type was not exported earlier, it is not possible that this change would affect users. It would be possible to break some users code if the type was exported locally, and not on the root level (some people might use the build file import directly to obtain the type information), but this fortunately was not the case.

@amandeepmittal amandeepmittal changed the title [notifications] rename and export getExpoPushTokenAsync parameter type [notifications][docs] rename and export getExpoPushTokenAsync parameter type Feb 7, 2023
@amandeepmittal
Copy link
Member

Updated the docs as per the latest conversation in the Slack thread with Will and Simek. Let me know your feedback on this.

cc: @Simek @kbrandwijk

@brentvatne
Copy link
Member

i'm about to cut sdk 48 docs, can you cherry-pick this to sdk-48 and ensure it's in sdk 48 docs when this ready?

@Simek Simek merged commit 829b33f into main Feb 9, 2023
@Simek Simek deleted the @simek/notification-export-missing-type branch February 9, 2023 17:31
@brentvatne brentvatne added the published Changes from the PR have been published to npm label Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: passed checks ExpoBot has nothing to complain about published Changes from the PR have been published to npm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants