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

@next/font/google: Hash filenames used in virtual file paths #2978

Merged
merged 3 commits into from Dec 13, 2022

Conversation

wbinnssmith
Copy link
Member

@wbinnssmith wbinnssmith commented Dec 12, 2022

Fixes WEB-289.

This uses hex-encoded hashes so that filenames for virtual assets for @next/font/google are unique. Previously, every virtual asset generated by @next/font/google would have the same path to the asset, impacting the quality of sourcemaps. This also keeps the basename consistent between the js and css module counterparts.

This also implements DeterministicHash for the &str type.

Test Plan: Given

import {Inter} from '@next/font/google';
import {Open_Sans} from '@next/font/google';

const inter = Inter({weight: "500"});
const sans = Open_Sans({weight: "500"});

...Chrome shows a tree of sourcemapped sources of:

[embedded_modules]/@vercel/turbopack-next
  internal
    font/google
      inter_34a0e3053298f029.js
      open_sans_a9468dae095a104a.js
      inter_34a0e3053298f029.module.css
      open_sans_a9468dae095a104a.module.css

and that the contents of each pair of basenames reflect the same imported font.

@wbinnssmith wbinnssmith requested a review from a team as a code owner December 12, 2022 18:38
@vercel
Copy link

vercel bot commented Dec 12, 2022

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

Name Status Preview Comments Updated
examples-nonmonorepo 🔄 Building (Inspect) Dec 13, 2022 at 1:41AM (UTC)
turbo-site ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Dec 13, 2022 at 1:41AM (UTC)
6 Ignored Deployments
Name Status Preview Comments Updated
examples-basic-web ⬜️ Ignored (Inspect) Dec 13, 2022 at 1:41AM (UTC)
examples-cra-web ⬜️ Ignored (Inspect) Dec 13, 2022 at 1:41AM (UTC)
examples-designsystem-docs ⬜️ Ignored (Inspect) Dec 13, 2022 at 1:41AM (UTC)
examples-kitchensink-blog ⬜️ Ignored (Inspect) Dec 13, 2022 at 1:41AM (UTC)
examples-native-web ⬜️ Ignored (Inspect) Dec 13, 2022 at 1:41AM (UTC)
examples-svelte-web ⬜️ Ignored (Inspect) Dec 13, 2022 at 1:41AM (UTC)

crates/next-core/src/next_font_google/mod.rs Outdated Show resolved Hide resolved
crates/next-core/src/next_font_google/mod.rs Outdated Show resolved Hide resolved
crates/next-core/src/next_font_google/mod.rs Outdated Show resolved Hide resolved
@wbinnssmith wbinnssmith force-pushed the wbinnssmith/use-hashed-filenames-virtual-assets branch 2 times, most recently from e3d8989 to 684c912 Compare December 12, 2022 20:20
@wbinnssmith wbinnssmith force-pushed the wbinnssmith/next-font-google-narrow-warning branch from 68efe7f to 5432699 Compare December 12, 2022 20:29
@wbinnssmith wbinnssmith force-pushed the wbinnssmith/use-hashed-filenames-virtual-assets branch from 684c912 to 2d9d332 Compare December 12, 2022 20:30
@wbinnssmith wbinnssmith force-pushed the wbinnssmith/next-font-google-narrow-warning branch from 5432699 to 5ae3f2a Compare December 12, 2022 22:23
@wbinnssmith wbinnssmith force-pushed the wbinnssmith/use-hashed-filenames-virtual-assets branch from 2d9d332 to cb3939d Compare December 12, 2022 22:23
@wbinnssmith wbinnssmith force-pushed the wbinnssmith/next-font-google-narrow-warning branch from 5ae3f2a to a51a973 Compare December 12, 2022 23:42
@wbinnssmith wbinnssmith force-pushed the wbinnssmith/use-hashed-filenames-virtual-assets branch from cb3939d to ba7be00 Compare December 12, 2022 23:42
@wbinnssmith wbinnssmith force-pushed the wbinnssmith/next-font-google-narrow-warning branch from a51a973 to 75bbdd5 Compare December 13, 2022 00:24
@wbinnssmith wbinnssmith force-pushed the wbinnssmith/use-hashed-filenames-virtual-assets branch from ba7be00 to a2d21a2 Compare December 13, 2022 00:24
@wbinnssmith wbinnssmith force-pushed the wbinnssmith/next-font-google-narrow-warning branch from 75bbdd5 to a3dc900 Compare December 13, 2022 00:52
@wbinnssmith wbinnssmith force-pushed the wbinnssmith/use-hashed-filenames-virtual-assets branch from a2d21a2 to aca84ad Compare December 13, 2022 00:52
@wbinnssmith wbinnssmith force-pushed the wbinnssmith/next-font-google-narrow-warning branch from a3dc900 to 1d886f6 Compare December 13, 2022 01:15
@wbinnssmith wbinnssmith force-pushed the wbinnssmith/use-hashed-filenames-virtual-assets branch from aca84ad to a20e32f Compare December 13, 2022 01:15
Base automatically changed from wbinnssmith/next-font-google-narrow-warning to main December 13, 2022 01:40
@github-actions
Copy link
Contributor

Benchmark for cab5e3e

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 8553.02µs ± 61.58µs 8493.09µs ± 47.17µs -0.70%
bench_hmr_to_commit/Turbopack RCC/1000 modules 8839.16µs ± 72.61µs 8807.87µs ± 70.65µs -0.35%
bench_hmr_to_commit/Turbopack RSC/1000 modules 868.47ms ± 27.24ms 905.78ms ± 13.30ms +4.30%
bench_hmr_to_commit/Turbopack SSR/1000 modules 8560.36µs ± 71.30µs 8564.55µs ± 57.93µs +0.05%
bench_hmr_to_eval/Turbopack CSR/1000 modules 7502.36µs ± 53.87µs 7557.45µs ± 89.07µs +0.73%
bench_hmr_to_eval/Turbopack RCC/1000 modules 7645.78µs ± 58.84µs 7599.51µs ± 70.11µs -0.61%
bench_hmr_to_eval/Turbopack SSR/1000 modules 7534.57µs ± 63.08µs 7544.18µs ± 56.75µs +0.13%
bench_hydration/Turbopack RCC/1000 modules 3563.75ms ± 27.91ms 3526.55ms ± 13.75ms -1.04%
bench_hydration/Turbopack RSC/1000 modules 2738.88ms ± 30.43ms 2713.19ms ± 30.08ms -0.94%
bench_hydration/Turbopack SSR/1000 modules 2992.94ms ± 24.80ms 3029.50ms ± 9.54ms +1.22%
bench_startup/Turbopack CSR/1000 modules 1454.50ms ± 4.17ms 1445.75ms ± 8.51ms -0.60%
bench_startup/Turbopack RCC/1000 modules 2684.06ms ± 34.35ms 2734.14ms ± 19.83ms +1.87%
bench_startup/Turbopack RSC/1000 modules 2354.58ms ± 39.85ms 2277.08ms ± 36.30ms -3.29%
bench_startup/Turbopack SSR/1000 modules 2325.20ms ± 24.85ms 2355.35ms ± 5.67ms +1.30%

@wbinnssmith wbinnssmith merged commit 5507548 into main Dec 13, 2022
@wbinnssmith wbinnssmith deleted the wbinnssmith/use-hashed-filenames-virtual-assets branch December 13, 2022 03:42
@github-actions
Copy link
Contributor

github-actions bot commented Dec 13, 2022

🟢 CI successful 🟢

Thanks

async fn get_request_id(query_vc: QueryMapVc) -> Result<StringVc> {
let query = &*query_vc.await?;
let query = query.as_ref().context("Query map must be present")?;
let mut to_hash = vec![];
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: There's no need to build a vec for this, using a Xxh3Hash64Hasher instance will allow you to compute this piecemeal.

Also, is this query sorted? We should consider ?a=b&c=d to be equal to ?c=d&a=b for hashing.

jridgewell pushed a commit to vercel/next.js that referenced this pull request Mar 10, 2023
…turbo#2978)

* Implement `DeterministicHash` for `&str`

* @next/font/google: Hash filenames used in virtual file paths

* Apply suggestions from code review

Co-authored-by: Leah <github.leah@hrmny.sh>

Co-authored-by: Leah <github.leah@hrmny.sh>
sokra pushed a commit to vercel/next.js that referenced this pull request Mar 13, 2023
…turbo#2978)

* Implement `DeterministicHash` for `&str`

* @next/font/google: Hash filenames used in virtual file paths

* Apply suggestions from code review

Co-authored-by: Leah <github.leah@hrmny.sh>

Co-authored-by: Leah <github.leah@hrmny.sh>
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.

None yet

3 participants