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 missing trailing / in commit artifacts workflow #31026

Merged
merged 1 commit into from
Sep 23, 2024

Conversation

poteto
Copy link
Member

@poteto poteto commented Sep 23, 2024

Stack from ghstack (oldest at bottom):

The trailing / was being omitted, so instead of moving the cjs
directory itself, it would move only its contents instead. This broke
some internal path assumptions.

Additionally, updates the step to create the react-dom directory prior
to moving.

[ghstack-poisoned]
Copy link

vercel bot commented Sep 23, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 23, 2024 6:55pm

poteto added a commit that referenced this pull request Sep 23, 2024
The trailing / was being omitted, so instead of moving the cjs
directory itself, it would move only its contents instead. This broke
some internal path assumptions.

ghstack-source-id: b6eedb0c88cd3aa3a786a3d3d280ede5ee81a063
Pull Request resolved: #31026
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Sep 23, 2024
@@ -118,14 +118,14 @@ jobs:
run: |
BASE_FOLDER='compiled-rn/facebook-fbsource/xplat/js'
mkdir -p ${BASE_FOLDER}/react-native-github/Libraries/Renderer/
mkdir -p ${BASE_FOLDER}/RKJSModules/vendor/react/{scheduler,react,react-is,react-test-renderer}/
mkdir -p ${BASE_FOLDER}/RKJSModules/vendor/react/{scheduler,react,react-dom,react-is,react-test-renderer}/
Copy link
Member

Choose a reason for hiding this comment

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

This line change doesn't match the description. Is that a second intentional change or accidental?

Copy link
Member Author

Choose a reason for hiding this comment

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

react-dom was added recently as a new directory in the rn sync but it had been omitted here. I'll update the pr summary

@react-sizebot
Copy link

Comparing: 4e9540e...2b62d2d

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.js = 6.68 kB 6.68 kB = 1.82 kB 1.82 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 509.10 kB 508.62 kB = 91.06 kB 90.98 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB = 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 514.04 kB 513.56 kB = 91.78 kB 91.71 kB
facebook-www/ReactDOM-prod.classic.js = 603.53 kB 603.01 kB = 106.78 kB 106.69 kB
facebook-www/ReactDOM-prod.modern.js = 579.76 kB 579.24 kB = 102.88 kB 102.79 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-experimental/react-dom/cjs/react-dom-unstable_testing.development.js = 973.12 kB 969.06 kB = 165.56 kB 165.02 kB
oss-experimental/react-dom/cjs/react-dom-profiling.development.js = 972.62 kB 968.57 kB = 164.57 kB 164.03 kB
oss-experimental/react-dom/cjs/react-dom-client.development.js = 956.18 kB 952.13 kB = 161.75 kB 161.20 kB
oss-experimental/react-reconciler/cjs/react-reconciler.development.js = 653.08 kB 649.02 kB = 104.91 kB 104.39 kB
oss-experimental/react-dom/cjs/react-dom-profiling.profiling.js = 551.33 kB 547.69 kB = 98.11 kB 97.59 kB
oss-experimental/react-art/cjs/react-art.development.js = 571.21 kB 567.16 kB = 92.39 kB 91.84 kB
oss-experimental/react-reconciler/cjs/react-reconciler.profiling.js = 423.50 kB 419.76 kB = 69.07 kB 68.62 kB

Generated by 🚫 dangerJS against 4033ff3

poteto added a commit that referenced this pull request Sep 23, 2024
The trailing / was being omitted, so instead of moving the cjs
directory itself, it would move only its contents instead. This broke
some internal path assumptions.

Additionally, updates the step to create the react-dom directory prior
to moving.

ghstack-source-id: b6eedb0c88cd3aa3a786a3d3d280ede5ee81a063
Pull Request resolved: #31026
@poteto poteto merged commit 4033ff3 into gh/poteto/6/base Sep 23, 2024
187 checks passed
poteto added a commit that referenced this pull request Sep 23, 2024
The trailing / was being omitted, so instead of moving the cjs
directory itself, it would move only its contents instead. This broke
some internal path assumptions.

Additionally, updates the step to create the react-dom directory prior
to moving.

ghstack-source-id: b6eedb0c88cd3aa3a786a3d3d280ede5ee81a063
Pull Request resolved: #31026
@poteto poteto deleted the gh/poteto/6/head branch September 23, 2024 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants