Skip to content

Commit 120d339

Browse files
committed
fix(turbopack): Allow google font fetch errors to propagate when in production
joshhannaford/pack-4667-google-font-fetch-should-fail-in-production-for-turbopack
1 parent 1f8abf1 commit 120d339

File tree

5 files changed

+39
-12
lines changed

5 files changed

+39
-12
lines changed

crates/next-core/src/next_font/google/mod.rs

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use turbo_tasks_hash::hash_xxh3_hash64;
1818
use turbopack::evaluate_context::node_evaluate_asset_context;
1919
use turbopack_core::{
2020
asset::AssetContent,
21+
chunk::ChunkingContext,
2122
context::AssetContext,
2223
ident::AssetIdent,
2324
issue::{IssueExt, IssueSeverity},
@@ -218,6 +219,7 @@ impl NextFontGoogleCssModuleReplacer {
218219
// requests to Google Fonts.
219220
let env = Vc::upcast::<Box<dyn ProcessEnv>>(CommandLineProcessEnv::new());
220221
let mocked_responses_path = &*env.read("NEXT_FONT_GOOGLE_MOCKED_RESPONSES".into()).await?;
222+
221223
let stylesheet_str = mocked_responses_path
222224
.as_ref()
223225
.map_or_else(
@@ -227,6 +229,7 @@ impl NextFontGoogleCssModuleReplacer {
227229
.await?;
228230

229231
let font_fallback = get_font_fallback(*self.project_path, options);
232+
let is_dev_mode = self.execution_context.chunking_context().is_dev_mode();
230233

231234
let stylesheet = match stylesheet_str {
232235
Some(s) => Some(
@@ -239,13 +242,21 @@ impl NextFontGoogleCssModuleReplacer {
239242
.owned()
240243
.await?,
241244
),
242-
None => {
245+
// Inform the user of the failure to retreive the stylesheet / font, but don't
246+
// propagate this error. We don't want e.g. offline connections to prevent page
247+
// renders during development. During production builds, however, this error
248+
// should propagate.
249+
None if *is_dev_mode.await? => {
243250
println!(
244251
"Failed to download `{}` from Google Fonts. Using fallback font instead.",
245252
options.await?.font_family
246253
);
247254
None
248255
}
256+
None => bail!(
257+
"Failed to fetch `{}` from Google Fonts.",
258+
options.await?.font_family
259+
),
249260
};
250261

251262
let css_asset = VirtualSource::new(
@@ -312,13 +323,20 @@ struct NextFontGoogleFontFileOptions {
312323
#[turbo_tasks::value(shared)]
313324
pub struct NextFontGoogleFontFileReplacer {
314325
project_path: ResolvedVc<FileSystemPath>,
326+
execution_context: ResolvedVc<ExecutionContext>,
315327
}
316328

317329
#[turbo_tasks::value_impl]
318330
impl NextFontGoogleFontFileReplacer {
319331
#[turbo_tasks::function]
320-
pub fn new(project_path: ResolvedVc<FileSystemPath>) -> Vc<Self> {
321-
Self::cell(NextFontGoogleFontFileReplacer { project_path })
332+
pub fn new(
333+
project_path: ResolvedVc<FileSystemPath>,
334+
execution_context: ResolvedVc<ExecutionContext>,
335+
) -> Vc<Self> {
336+
Self::cell(NextFontGoogleFontFileReplacer {
337+
project_path,
338+
execution_context,
339+
})
322340
}
323341
}
324342

@@ -616,16 +634,9 @@ async fn fetch_from_google_fonts(
616634
)
617635
.await?;
618636

619-
Ok(match &*result {
637+
Ok(match *result {
620638
Ok(r) => Some(*r.await?.body),
621639
Err(err) => {
622-
// Inform the user of the failure to retreive the stylesheet / font, but don't
623-
// propagate this error. We don't want e.g. offline connections to prevent page
624-
// renders during development. During production builds, however, this error
625-
// should propagate.
626-
//
627-
// TODO(WEB-283): Use fallback in dev in this case
628-
// TODO(WEB-293): Fail production builds (not dev) in this case
629640
err.to_issue(IssueSeverity::Warning.into(), virtual_path)
630641
.to_resolved()
631642
.await?

crates/next-core/src/next_import_map.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -902,7 +902,7 @@ async fn insert_next_shared_aliases(
902902
import_map.insert_alias(
903903
AliasPattern::exact(GOOGLE_FONTS_INTERNAL_PREFIX),
904904
ImportMapping::Dynamic(ResolvedVc::upcast(
905-
NextFontGoogleFontFileReplacer::new(*project_path)
905+
NextFontGoogleFontFileReplacer::new(*project_path, execution_context)
906906
.to_resolved()
907907
.await?,
908908
))

turbopack/crates/turbopack-browser/src/chunking_context.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -705,4 +705,11 @@ impl ChunkingContext for BrowserChunkingContext {
705705
self.chunk_item_id_from_ident(AsyncLoaderModule::asset_ident_for(module))
706706
})
707707
}
708+
709+
#[turbo_tasks::function]
710+
async fn is_dev_mode(self: Vc<Self>) -> Result<Vc<bool>> {
711+
Ok(Vc::cell(
712+
self.await?.runtime_type == RuntimeType::Development,
713+
))
714+
}
708715
}

turbopack/crates/turbopack-core/src/chunk/chunking_context.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,8 @@ pub trait ChunkingContext {
249249
fn chunk_item_id_from_module(self: Vc<Self>, module: Vc<Box<dyn Module>>) -> Vc<ModuleId> {
250250
self.chunk_item_id_from_ident(module.ident())
251251
}
252+
253+
fn is_dev_mode(self: Vc<Self>) -> Result<Vc<bool>>;
252254
}
253255

254256
pub trait ChunkingContextExt {

turbopack/crates/turbopack-nodejs/src/chunking_context.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,4 +494,11 @@ impl ChunkingContext for NodeJsChunkingContext {
494494
self.chunk_item_id_from_ident(AsyncLoaderModule::asset_ident_for(module))
495495
})
496496
}
497+
498+
#[turbo_tasks::function]
499+
async fn is_dev_mode(self: Vc<Self>) -> Result<Vc<bool>> {
500+
Ok(Vc::cell(
501+
self.await?.runtime_type == RuntimeType::Development,
502+
))
503+
}
497504
}

0 commit comments

Comments
 (0)