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

[Bug]: Typescript errors are getting shipped #2333

Closed
jrobber opened this issue Oct 25, 2022 · 2 comments · Fixed by #2334
Closed

[Bug]: Typescript errors are getting shipped #2333

jrobber opened this issue Oct 25, 2022 · 2 comments · Fixed by #2334

Comments

@jrobber
Copy link

jrobber commented Oct 25, 2022

Mapbox Implementation

Mapbox

Mapbox Version

10.0.0-beta.49

Platform

iOS, Android

@rnmapbox/maps version

10.0.0-beta.11

Standalone component to reproduce

Not applicable

Observed behavior and steps to reproduce

pnpm tsc fails in my project because I use different settings.

The extra setting I use is:

    "noUnusedLocals": true,

Expected behavior

A) According to the Typescript gurus rnmapbox should be shipping JS files not TS files directly:
microsoft/TypeScript#47387 (comment)

exclude on my end will not fix this.

OR

B) rnmapbox should enable every typescript setting and fix all errors.

Notes / preliminary analysis

I forked the repo and added the flag and it was pretty quick to clean up all the errors.

And then... I tried to commit and found you have a generator that runs that generates your styles.
It's currently generating 12 unused Enums and types in MapboxStyles.ts.

I followed it and then decided that I should check with the team before I change the .json file that provides the styles to make sure that the fix is in the right direction.

I could also setup a build process and update the main in package.json to use the compiled files.

I realize that I am using a beta project. I feel like this should be addressed though because:

  • Expo 46 relies on mapbox v10+

So my choices are to use mapbox v10+ and have these errors in my repo, or to not use mapbox because v8 isn't working on expo 46 anymore.

Also, v10+ has been in beta for a long time. If it's going to stay in beta then a simple build process to fix these issues seems reasonable.

Additional links and references

No response

@YousefED
Copy link
Contributor

Running into the same issues. I think the root of this is what you describe in (A) and what's further discussed in #2172

Related issues:

@mfazekas
Copy link
Contributor

@jrobber thanks much for the great description of the issue. You describe a concrete issue rather than arguing that in general it's a good idea to precompile ts files.

As noted on #2172, I don't mind precompiling when creating the npm package. But we don't want generated js files in git source, and also metro should keep using the ts source files react-native key in pacakges.json.

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 a pull request may close this issue.

3 participants