Skip to content

feat(cdn): use single fingerprint for all files in a package #150

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

Merged
merged 4 commits into from
Mar 20, 2025

Conversation

sgore-godaddy
Copy link
Contributor

Summary

Introducing an option to have a single fingerprint id for all the files in a package so that the files can be co-located at runtime.

Using https://github.com/paralleldrive/cuid2 to generate secure, collision-resistant ids.

Changelog

Test Plan

Tests are updated and confirmed to be passing.

@sgore-godaddy sgore-godaddy force-pushed the use-single-fingerprint branch from 2e390de to 5a0050b Compare March 11, 2025 19:32
let fingerPrintId;

if (useSingleFingerprint) {
fingerPrintId = createId();
Copy link
Contributor

@mswaagman-godaddy mswaagman-godaddy Mar 13, 2025

Choose a reason for hiding this comment

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

Fingerprints used to be based on file contents, why not take the combined fingerprints of each file and generate a fingerprint of the fingerprints? Considering this is just to cache bust browsers, an accidental UUID collision over time shouldn't be a worry IMHO.

edit: the downside of a collision-resistant unique fingerprint is that it would still cache bust if contents of published packages are identical, ideally it wouldn't in that case.

Copy link
Contributor Author

@sgore-godaddy sgore-godaddy Mar 13, 2025

Choose a reason for hiding this comment

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

Ok this makes sense, Thanks for the review.

I have refactored the changes to generate a single ID when needed by combining individual IDs.
I could also re-instate the previous test with only a small modifications which guarantees new changes are not going to break the existing logic. I have also simplified the new test as now we know based on the contents what is the exact fingerprint it will be generating.

@sgore-godaddy sgore-godaddy force-pushed the use-single-fingerprint branch from 18f8cd6 to 832492a Compare March 13, 2025 13:36
@sgore-godaddy sgore-godaddy merged commit c62d7e2 into godaddy:main Mar 20, 2025
1 check passed
@sgore-godaddy sgore-godaddy deleted the use-single-fingerprint branch March 20, 2025 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants