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

chore: rename normalize-color to normalize-colors (umbrella 480) #34571

Closed

Conversation

Titozzz
Copy link
Collaborator

@Titozzz Titozzz commented Sep 2, 2022

Summary

Changelog

[General] [Changed] - Rename normalize-color to normalize-colors as part of react-native-community/discussions-and-proposals#480

Test Plan

@Titozzz Titozzz requested a review from hramos as a code owner September 2, 2022 09:02
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 2, 2022
@Titozzz Titozzz changed the title chore: rename normalize-color to normalize-colors (umbrella discussio… chore: rename normalize-color to normalize-colors (umbrella 480) Sep 2, 2022
@github-actions
Copy link

github-actions bot commented Sep 2, 2022

Warnings
⚠️ 🔒 package.json - Changes were made to package.json. This will require a manual import by a Facebook employee.

Generated by 🚫 dangerJS against d7b39e7

@facebook-github-bot facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Sep 2, 2022
@Titozzz Titozzz force-pushed the chore-480-rename-normalize-color branch from 7f1417b to 85f87b4 Compare September 2, 2022 09:04
@facebook-github-bot
Copy link
Contributor

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

1 similar comment
@facebook-github-bot
Copy link
Contributor

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

@analysis-bot
Copy link

analysis-bot commented Sep 2, 2022

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,103,618 +57
android hermes armeabi-v7a 6,471,949 +63
android hermes x86 7,521,258 +55
android hermes x86_64 7,380,136 +55
android jsc arm64-v8a 8,971,262 +2,863
android jsc armeabi-v7a 7,702,367 +2,862
android jsc x86 9,033,571 +2,865
android jsc x86_64 9,511,602 +2,853

Base commit: 2d1d61a
Branch: main

@analysis-bot
Copy link

analysis-bot commented Sep 2, 2022

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

Base commit: 2d1d61a
Branch: main

@kelset kelset added the Tech: Monorepo For PRs that are related to the monorepo infra label Sep 12, 2022
@cortinico cortinico mentioned this pull request Sep 14, 2022
11 tasks
@@ -2,7 +2,7 @@ load("@fbsource//tools/build_defs/third_party:yarn_defs.bzl", "yarn_workspace")
load("@fbsource//xplat/js:JS_DEFS.bzl", "rn_library")

rn_library(
name = "normalize-color",
Copy link
Contributor

Choose a reason for hiding this comment

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

Commenting here but valid for the whole PR:

Let's undo the BUCK update and the path change. It will make easier for us to import and merge this change 👍

I've also posted an update on the monorepo effort there:

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done ✅

@Titozzz Titozzz force-pushed the chore-480-rename-normalize-color branch from 02bb773 to 4abf495 Compare September 23, 2022 12:13
BUCK Outdated
@@ -769,7 +769,7 @@ rn_library(
"//xplat/js/RKJSModules/vendor/react-test-renderer:react-test-renderer",
"//xplat/js/RKJSModules/vendor/scheduler:scheduler",
"//xplat/js/react-native-github/packages/assets:assets",
"//xplat/js/react-native-github/packages/normalize-color:normalize-color",
"//xplat/js/react-native-github/packages/normalize-colors:normalize-color",
Copy link
Contributor

Choose a reason for hiding this comment

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

This should also be reverted.

"description": "Color normalization for React Native.",
"repository": {
"type": "git",
"url": "git@github.com:facebook/react-native.git",
"directory": "packages/normalize-color"
"directory": "packages/normalize-colors"
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be reverted, alongside the folder renaming

@facebook-github-bot
Copy link
Contributor

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

1 similar comment
@facebook-github-bot
Copy link
Contributor

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

@cipolleschi
Copy link
Contributor

could you rebase this? I'll try to cary this out whenever I have some time.

@hoxyq hoxyq force-pushed the chore-480-rename-normalize-color branch from e874737 to d5554bd Compare November 25, 2022 09:30
@pull-bot
Copy link

PR build artifact for d5554bd is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@pull-bot
Copy link

PR build artifact for d5554bd is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@facebook-github-bot
Copy link
Contributor

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

@pull-bot
Copy link

PR build artifact for d7b39e7 is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@pull-bot
Copy link

PR build artifact for d7b39e7 is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@facebook-github-bot
Copy link
Contributor

@hoxyq 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 @Titozzz in dc33559.

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 Dec 1, 2022
@yungsters
Copy link
Contributor

Should we have also renamed packages/normalize-color{ => s}?

@hoxyq
Copy link
Contributor

hoxyq commented Dec 13, 2022

Should we have also renamed packages/normalize-color{ => s}?

We plan to do all renamings in Phase 4, as discussed in original RFC: react-native-community/discussions-and-proposals#480

OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
…ebook#34571)

Summary:
## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[General] [Changed] - Rename normalize-color to normalize-colors as part of react-native-community/discussions-and-proposals#480

Pull Request resolved: facebook#34571

Reviewed By: cortinico

Differential Revision: D39235696

Pulled By: hoxyq

fbshipit-source-id: b6d5fcae9fb5c953c2f7b48f73a95cd883ff8f63
facebook-github-bot pushed a commit that referenced this pull request 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 change 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.

Pull Request resolved: #40971

Test Plan: CI

Reviewed By: cipolleschi

Differential Revision: D50298291

Pulled By: robhogan

fbshipit-source-id: 4bf6503108335ffa52654346d1874c217071ff91
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. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. Tech: Monorepo For PRs that are related to the monorepo infra
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants