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

[CI] Verdaccio: proxy @react-native/normalize-colors from NPM #40971

Closed
wants to merge 1 commit into from

Conversation

robhogan
Copy link
Contributor

@robhogan robhogan commented Oct 14, 2023

Summary:

To address the root cause of a recurring issue (#40797, #39692) where breaking changes to @react-native/normalize-colors would be pulled into old versions of deprecated-react-native-prop-types, we recently changed the dependency in the latter to use a semver range (facebook/react-native-deprecated-modules#27, #40869).

For CI, we generally force @react-native/* to be resolved only from Verdaccio locally published packages - ie, the current versions at source. The source version (currently 0.74.1) isn't semver-compatible with deprecated-react-native-prop-types's dependency (^0.73.0), so npm install was failing in CI with "no package found". We should be getting 0.73.2 from the public registry in this case.

This restores a previous workaround added in #34571 but not updated since facebook/react-native-deprecated-modules#11 meant the dependency was now on the pluralised package. We have no dependency on the old non-plural package any more.

Changelog:

[INTERNAL] [FIXED] - CI/Verdaccio: Proxy @react-native/normalize-colors from NPM for the deprecated-react-native-prop-types dependency.

Test Plan:

CI

@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: Facebook Partner: Facebook Partner labels Oct 14, 2023
@facebook-github-bot
Copy link
Contributor

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

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 17,214,614 +6
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 20,587,381 +1
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: 9497203
Branch: main

@hoxyq
Copy link
Contributor

hoxyq commented Oct 14, 2023

We have no dependency on the old non-plural package any more.

Then we probably don't need this rule inside Verdaccio config.

Looking at https://verdaccio.org/docs/packages, can you try specifying publish: $all for this package?

@github-actions
Copy link

This pull request was successfully merged by @robhogan in d282ebe.

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

@github-actions github-actions bot added the Merged This PR has been merged. label Oct 14, 2023
@robhogan
Copy link
Contributor Author

@hoxyq I believe we need the rule in order to allow the transitive dependency on ^0.73.0 to be satisfied, because we don't publish any package satisfying that range to Verdaccio. The reasoning is the same as it was with the singular package - the workaround just wasn't updated. I'm not sure I understand what publish: $all would achieve?

One alternative would be to publish a patched version of deprecated-react-native-prop-types to local Verdaccio which is allowed to depend on the latest source version of @react-native/normalized-colors, or achieve something similar by adding resolutions to the CI test template.

@robhogan robhogan deleted the fix/ci-normalize-colors branch October 14, 2023 14:23
@robhogan
Copy link
Contributor Author

I see - some CI jobs rely pulling @react-native/normalize-colors@^0.73.0 from the public registry, and others rely on publishing @react-native/normalize-colors@0.74.1 to the local registry. This PR fixed the former and broke the latter.

I'm not familiar enough with Verdaccio to know if publish: $all will achieve what we want here (is it possible to pull some versions of a package from NPM and other versions of the same package from Verdaccio?) but feels like it's worth a shot - running in #40972.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants