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

Build: use content hash for facebook-react-native build #27577

Merged
merged 1 commit into from
Oct 25, 2023

Conversation

kassens
Copy link
Member

@kassens kassens commented Oct 24, 2023

Similar to #26734, this switches the RN builds for Meta to a content hash instead of git commit number to make the builds reproducible and avoid creating sync commits if the bundled content didn't change.

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Oct 24, 2023
@react-sizebot
Copy link

Comparing: 960ed6e...f5a2de5

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.min.js = 175.04 kB 175.04 kB = 54.46 kB 54.47 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 177.17 kB 177.17 kB = 55.15 kB 55.15 kB
facebook-www/ReactDOM-prod.classic.js = 567.61 kB 567.61 kB = 99.94 kB 99.94 kB
facebook-www/ReactDOM-prod.modern.js = 551.47 kB 551.47 kB = 97.04 kB 97.04 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against f5a2de5

Comment on lines +359 to +366
function hashJSFilesInDirectory(directory) {
const hash = crypto.createHash('sha1');
for (const filePath of glob.sync(directory + '/**/*.js').sort()) {
hash.update(fs.readFileSync(filePath));
}
return hash.digest('hex').slice(0, 8);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we hash based on only JS files? Not important now, but hashing on non-directories seems more reliable, in case we will distribute non-js files at some point

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm actually not sure if we even need this version string anyway? I think devtools use them in some capacity, but I'm not sure what the exact requirements are.

@kassens kassens merged commit 51ffd35 into facebook:main Oct 25, 2023
36 checks passed
@kassens kassens deleted the content-hash branch October 25, 2023 13:50
github-actions bot pushed a commit that referenced this pull request Oct 25, 2023
Similar to #26734, this switches the RN builds for Meta to a content
hash instead of git commit number to make the builds reproducible and
avoid creating sync commits if the bundled content didn't change.

DiffTrain build for [51ffd35](51ffd35)
@noahlemen
Copy link
Member

avoid creating sync commits if the bundled content didn't change

nice! thanks for doing this

kassens added a commit that referenced this pull request Oct 25, 2023
kassens added a commit that referenced this pull request Oct 25, 2023
)

Reverts #27577.

We also sync React Native OSS bundles which means this didn't work as
hoped unless we abandon the commit hash in OSS which seems useful.
github-actions bot pushed a commit that referenced this pull request Oct 25, 2023
)

Reverts #27577.

We also sync React Native OSS bundles which means this didn't work as
hoped unless we abandon the commit hash in OSS which seems useful.

DiffTrain build for [97047a8](97047a8)
timneutkens pushed a commit to vercel/next.js that referenced this pull request Oct 25, 2023
React upstream changes:

- facebook/react#27586
- facebook/react#27577
- facebook/react#27576

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
jerrydev0927 added a commit to jerrydev0927/react that referenced this pull request Jan 5, 2024
…584)

Reverts facebook/react#27577.

We also sync React Native OSS bundles which means this didn't work as
hoped unless we abandon the commit hash in OSS which seems useful.

DiffTrain build for [97047a810cb19e3041a3ec163f4fde85f495bdc1](facebook/react@97047a8)
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
Similar to facebook#26734, this switches the RN builds for Meta to a content
hash instead of git commit number to make the builds reproducible and
avoid creating sync commits if the bundled content didn't change.
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
…ebook#27584)

Reverts facebook#27577.

We also sync React Native OSS bundles which means this didn't work as
hoped unless we abandon the commit hash in OSS which seems useful.
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
Similar to #26734, this switches the RN builds for Meta to a content
hash instead of git commit number to make the builds reproducible and
avoid creating sync commits if the bundled content didn't change.

DiffTrain build for commit 51ffd35.
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