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

Update forked dependency to fix typescript error #34422

Merged
merged 6 commits into from
Aug 31, 2021

Conversation

guarani
Copy link
Contributor

@guarani guarani commented Aug 31, 2021

Description

Fixes #34377

The TypeScript dependency was not pinned to a specific version which allowed it to fetch changes that brought in breaking changes to downstream clients of this library.

h/t to @ceyhun for figuring this one out.

Related:

How has this been tested?

⚠️ All CI checks should pass on this PR except for the Compressed Size and Performances Tests jobs, which will still fail because they test trunk (which is currently broken). So if all other tests pass, the proposal here is to merge this PR despite these two failing tests. This will allow trunk to work again.

Types of changes

  • Tooling bug fix

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

@guarani guarani added the Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) label Aug 31, 2021
@guarani guarani requested a review from ceyhun August 31, 2021 14:47
Copy link
Member

@ceyhun ceyhun left a comment

Choose a reason for hiding this comment

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

I tested this locally by running git clean -fdx && npm cache clean --force && npm ci in trunk first, which failed with the same TS error in #34377.
Then I ran the same command in this branch and it was successful 🎉

@guarani guarani marked this pull request as ready for review August 31, 2021 17:32
@gziolo gziolo merged commit 4813ac8 into trunk Aug 31, 2021
@gziolo gziolo deleted the rnmobile/fix/typescript-version-in-forked-dependency branch August 31, 2021 17:38
@github-actions github-actions bot added this to the Gutenberg 11.5 milestone Aug 31, 2021
noisysocks pushed a commit that referenced this pull request Sep 1, 2021
* Update forked dependency to fix typescript error

* Temporary: create new npm cache

* Point to react-native-gesture-handler fork tag

* Temporary: new npm cache for build-plugin-zip.sh

* Revert "Temporary: new npm cache for build-plugin-zip.sh"

This reverts commit b04adb2.

* Revert "Temporary: create new npm cache"

This reverts commit ab60daa.
@ramonjd
Copy link
Member

ramonjd commented Sep 1, 2021

I've 🍒 picked this PR into today's 11.4 release since the build was broken there too. Thank you!

@ramonjd ramonjd modified the milestones: Gutenberg 11.5, Gutenberg 11.4 Sep 1, 2021
@ramonjd ramonjd mentioned this pull request Sep 1, 2021
1 task
ramonjd pushed a commit that referenced this pull request Sep 1, 2021
* Update forked dependency to fix typescript error

* Temporary: create new npm cache

* Point to react-native-gesture-handler fork tag

* Temporary: new npm cache for build-plugin-zip.sh

* Revert "Temporary: new npm cache for build-plugin-zip.sh"

This reverts commit b04adb2.

* Revert "Temporary: create new npm cache"

This reverts commit ab60daa.
ramonjd pushed a commit that referenced this pull request Sep 2, 2021
* Update forked dependency to fix typescript error

* Temporary: create new npm cache

* Point to react-native-gesture-handler fork tag

* Temporary: new npm cache for build-plugin-zip.sh

* Revert "Temporary: new npm cache for build-plugin-zip.sh"

This reverts commit b04adb2.

* Revert "Temporary: create new npm cache"

This reverts commit ab60daa.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build: npm install fails on CI because of react-native-gesture-handler
5 participants