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

Fix codegen trying to parse .d.ts files #34439

Closed
wants to merge 1 commit into from
Closed

Conversation

tido64
Copy link
Collaborator

@tido64 tido64 commented Aug 17, 2022

Summary

With react-native 0.70-rc.3 and new arch, codegen may fail if it encounters .d.ts files because specs may appear to be unused.

Changelog

[General] [Fixed] - Codegen should ignore .d.ts files

Test Plan

See repro in microsoft/react-native-test-app#1052. The build will fail without manually patching this in.

If you prefer to use your own test app, try adding react-native-safe-area-context as a dependency.

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Microsoft Partner: Microsoft Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Aug 17, 2022
@kelset
Copy link
Contributor

kelset commented Aug 17, 2022

Looks like we might have to backport this to 0.68 and 0.69 potentially? What do you think @cortinico? It will require a new release of the codegen tool

(for sure we'll need to cherry-pick for 0.70, I'll add it to the road to 0.70)

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 4706d13
Branch: main

@cortinico
Copy link
Contributor

/rebase

@cortinico
Copy link
Contributor

Looks like we might have to backport this to 0.68 and 0.69 potentially? What do you think @cortinico? It will require a new release of the codegen tool

Yes it will require a new release of the codegen, to potentially be backported also.

For the future we could think about having a: // react-native-codegen-ignore comment that allows to skip single files if needed (as now we pick everything based on the name).

@cortinico
Copy link
Contributor

/rebase

This failed due to permissions, Need to look into it.
If you rebase your PR though, the CI should be green

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,614,917 +61
android hermes armeabi-v7a 7,029,837 +52
android hermes x86 7,915,630 +41
android hermes x86_64 7,889,364 +54
android jsc arm64-v8a 9,493,834 +49
android jsc armeabi-v7a 8,271,707 +46
android jsc x86 9,431,917 +36
android jsc x86_64 10,024,969 +52

Base commit: 4eec473
Branch: main

@facebook-github-bot
Copy link
Contributor

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @tido64 in 0f0d520.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Aug 18, 2022
@kelset kelset deleted the tido/ignore-d.ts-files branch August 22, 2022 09:45
kelset pushed a commit that referenced this pull request Aug 22, 2022
Summary:
With react-native 0.70-rc.3 and new arch, codegen may fail if it encounters `.d.ts` files because specs may appear to be unused.

## Changelog

[General] [Fixed] - Codegen should ignore `.d.ts` files

Pull Request resolved: #34439

Test Plan:
See repro in microsoft/react-native-test-app#1052. The build will fail without manually patching this in.

If you prefer to use your own test app, try adding [react-native-safe-area-context](https://github.com/th3rdwave/react-native-safe-area-context) as a dependency.

Reviewed By: cipolleschi

Differential Revision: D38826388

Pulled By: cortinico

fbshipit-source-id: eb7c9be2d49286bae86b2428862fbf20f6f32ca5
kelset pushed a commit that referenced this pull request Aug 24, 2022
Summary:
With react-native 0.70-rc.3 and new arch, codegen may fail if it encounters `.d.ts` files because specs may appear to be unused.

## Changelog

[General] [Fixed] - Codegen should ignore `.d.ts` files

Pull Request resolved: #34439

Test Plan:
See repro in microsoft/react-native-test-app#1052. The build will fail without manually patching this in.

If you prefer to use your own test app, try adding [react-native-safe-area-context](https://github.com/th3rdwave/react-native-safe-area-context) as a dependency.

Reviewed By: cipolleschi

Differential Revision: D38826388

Pulled By: cortinico

fbshipit-source-id: eb7c9be2d49286bae86b2428862fbf20f6f32ca5
Titozzz pushed a commit that referenced this pull request Oct 10, 2022
Summary:
With react-native 0.70-rc.3 and new arch, codegen may fail if it encounters `.d.ts` files because specs may appear to be unused.

## Changelog

[General] [Fixed] - Codegen should ignore `.d.ts` files

Pull Request resolved: #34439

Test Plan:
See repro in microsoft/react-native-test-app#1052. The build will fail without manually patching this in.

If you prefer to use your own test app, try adding [react-native-safe-area-context](https://github.com/th3rdwave/react-native-safe-area-context) as a dependency.

Reviewed By: cipolleschi

Differential Revision: D38826388

Pulled By: cortinico

fbshipit-source-id: eb7c9be2d49286bae86b2428862fbf20f6f32ca5
diegolmello pushed a commit to RocketChat/react-native that referenced this pull request Feb 2, 2023
Summary:
With react-native 0.70-rc.3 and new arch, codegen may fail if it encounters `.d.ts` files because specs may appear to be unused.

## Changelog

[General] [Fixed] - Codegen should ignore `.d.ts` files

Pull Request resolved: facebook#34439

Test Plan:
See repro in microsoft/react-native-test-app#1052. The build will fail without manually patching this in.

If you prefer to use your own test app, try adding [react-native-safe-area-context](https://github.com/th3rdwave/react-native-safe-area-context) as a dependency.

Reviewed By: cipolleschi

Differential Revision: D38826388

Pulled By: cortinico

fbshipit-source-id: eb7c9be2d49286bae86b2428862fbf20f6f32ca5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. p: Microsoft Partner: Microsoft Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants