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

[expo]: Add types folder to published package #31339

Merged
merged 2 commits into from
Sep 9, 2024

Conversation

marklawlor
Copy link
Contributor

@marklawlor marklawlor commented Sep 5, 2024

Why

Add types folder to package.json. This folder should be included for better typing in Expo Web.

Running the CLI with TypeScript enabled will create a expo-env.d.ts that references this folder. This has gone unnoticed as there is no warning/error and these types are additive (and also supplied by router) but VS Code now has a warning that the folder is missing.

Fixes #31250

How

Test Plan

Checklist

Sorry, something went wrong.

@expo-bot
Copy link
Collaborator

expo-bot commented Sep 5, 2024

The Pull Request introduced fingerprint changes against the base commit: df216d9

Fingerprint diff
[
  {
    "op": "changed",
    "source": {
      "type": "dir",
      "filePath": "../../node_modules/expo",
      "reasons": [
        "bareRncliAutolinking"
      ],
      "hash": "41d8db5d77625f15779c4d1ded50d4a28736a822"
    }
  },
  {
    "op": "changed",
    "source": {
      "type": "dir",
      "filePath": "../../packages/expo",
      "reasons": [
        "expoAutolinkingIos",
        "expoAutolinkingAndroid",
        "expoConfigPlugins",
        "expoConfigPlugins"
      ],
      "hash": "a34dbe8f85b79ebb2d509e5009e10f70e7c5bf58"
    }
  }
]

Generated by PR labeler 🤖

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Co-authored-by: Expo Bot <34669131+expo-bot@users.noreply.github.com>
@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Sep 5, 2024
Copy link
Contributor

@Kudo Kudo left a comment

Choose a reason for hiding this comment

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

looks good to me but @byCedric mentioned some concern that whether these types are internal and should not publish publicly. please double check if we need all these types publicly. see https://exponent-internal.slack.com/archives/C5ERY0TAR/p1725439343314989?thread_ts=1725411493.294319&cid=C5ERY0TAR

@Kudo Kudo requested a review from byCedric September 5, 2024 11:25
@marklawlor
Copy link
Contributor Author

I don't consider any of these types internal. These are not types for the expo package. They just improve the types of the existing user environment. This should only affect existing globals (require()) or imports from react-native-web

@Kudo
Copy link
Contributor

Kudo commented Sep 6, 2024

I don't consider any of these types internal. These are not types for the expo package. They just improve the types of the existing user environment. This should only affect existing globals (require()) or imports from react-native-web

it looks like we've tried to override react-native types:

declare module 'react-native' {

not sure if that would conflict with react-native types?

@marklawlor
Copy link
Contributor Author

marklawlor commented Sep 9, 2024

@Kudo You cannot override with TypeScript, interfaces are always merged. If there is a conflict (e.g impossible to satisfy unions), TypeScript will error and fail to compile.

For example, with these changes ViewProps will be a union of the published types and our definitions.

@marklawlor marklawlor merged commit 21472e0 into main Sep 9, 2024
19 checks passed
@marklawlor marklawlor deleted the @marklawlor/expo/missing-types-folder branch September 9, 2024 05:52
@Kudo
Copy link
Contributor

Kudo commented Sep 9, 2024

cool, thanks for clarifying that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: fingerprint changed bot: passed checks ExpoBot has nothing to complain about
Projects
None yet
Development

Successfully merging this pull request may close these issues.

expo/types is not published
4 participants