Skip to content

Commit 4e43a45

Browse files
lukesandberglukesandberg
committed
address some code review feedback
1 parent 0b1b0ed commit 4e43a45

File tree

6 files changed

+48
-52
lines changed

6 files changed

+48
-52
lines changed

‎crates/next-core/src/next_client/transforms.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ pub async fn get_next_client_transforms_rules(
5858
pages_dir.clone(),
5959
ExportFilter::StripDataExports,
6060
enable_mdx_rs,
61-
vec![],
6261
)?);
6362
rules.push(get_next_disallow_export_all_in_page_rule(
6463
enable_mdx_rs,

‎crates/next-core/src/next_pages/page_entry.rs

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,12 @@ pub async fn create_page_ssr_entry_module(
7878
("VAR_USERLAND", &inner),
7979
];
8080

81-
if reference_type == ReferenceType::Entry(EntryReferenceSubType::Page)
82-
|| reference_type == ReferenceType::Entry(EntryReferenceSubType::PageData)
83-
{
81+
let is_page = matches!(
82+
reference_type,
83+
ReferenceType::Entry(EntryReferenceSubType::Page)
84+
| ReferenceType::Entry(EntryReferenceSubType::PageData)
85+
);
86+
if is_page {
8487
replacements.push(("VAR_MODULE_DOCUMENT", &inner_document));
8588
replacements.push(("VAR_MODULE_APP", &inner_app));
8689
}
@@ -92,8 +95,7 @@ pub async fn create_page_ssr_entry_module(
9295
// When we're building the instrumentation page (only when the
9396
// instrumentation file conflicts with a page also labeled
9497
// /instrumentation) hoist the `register` method.
95-
if (reference_type == ReferenceType::Entry(EntryReferenceSubType::Page)
96-
|| reference_type == ReferenceType::Entry(EntryReferenceSubType::PageData))
98+
if is_page
9799
&& (definition_page == "/instrumentation" || definition_page == "/src/instrumentation")
98100
{
99101
let file = &*file_content_rope(source.content().file_content()).await?;
@@ -108,8 +110,8 @@ pub async fn create_page_ssr_entry_module(
108110

109111
let file = File::from(result.build());
110112

111-
source = Vc::upcast(VirtualSource::new(
112-
source.ident().path().owned().await?,
113+
source = Vc::upcast(VirtualSource::new_with_ident(
114+
source.ident(),
113115
AssetContent::file(file.into()),
114116
));
115117
}
@@ -120,10 +122,10 @@ pub async fn create_page_ssr_entry_module(
120122

121123
let pages_structure_ref = pages_structure.await?;
122124

123-
let (app_module, document_module) = if reference_type
124-
== ReferenceType::Entry(EntryReferenceSubType::Page)
125-
|| reference_type == ReferenceType::Entry(EntryReferenceSubType::PageData)
126-
{
125+
let (app_module, document_module) = if is_page {
126+
// TODO: should we process the document and app modules in the same ReferenceType?
127+
// if could pick a different static reference type then we could share the document and app
128+
// modules across Pages and PagesData.
127129
let document_module = process_global_item(
128130
*pages_structure_ref.document,
129131
reference_type.clone(),
@@ -153,9 +155,7 @@ pub async fn create_page_ssr_entry_module(
153155
.module();
154156

155157
if matches!(runtime, NextRuntime::Edge) {
156-
if reference_type == ReferenceType::Entry(EntryReferenceSubType::Page)
157-
|| reference_type == ReferenceType::Entry(EntryReferenceSubType::PageData)
158-
{
158+
if is_page {
159159
ssr_module = wrap_edge_page(
160160
ssr_module_context,
161161
project_root,

‎crates/next-core/src/next_server/transforms.rs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,7 @@ use next_custom_transforms::transforms::strip_page_exports::ExportFilter;
33
use turbo_rcstr::RcStr;
44
use turbo_tasks::{ResolvedVc, Vc};
55
use turbopack::module_options::{ModuleRule, ModuleRuleEffect, RuleCondition};
6-
use turbopack_core::reference_type::{
7-
CssReferenceSubType, EntryReferenceSubType, ReferenceType, UrlReferenceSubType,
8-
};
6+
use turbopack_core::reference_type::{CssReferenceSubType, ReferenceType, UrlReferenceSubType};
97

108
use crate::{
119
mode::NextMode,
@@ -81,9 +79,6 @@ pub async fn get_next_server_transforms_rules(
8179
pages_dir.clone(),
8280
ExportFilter::StripDefaultExport,
8381
mdx_rs,
84-
vec![RuleCondition::ReferenceType(ReferenceType::Entry(
85-
EntryReferenceSubType::PageData,
86-
))],
8782
)?);
8883
}
8984
false

‎crates/next-core/src/next_shared/transforms/mod.rs

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ pub async fn get_next_image_rule() -> Result<ModuleRule> {
7171
))
7272
}
7373

74-
fn match_js_extension(enable_mdx_rs: bool) -> Vec<RuleCondition> {
74+
fn match_js_extension(enable_mdx_rs: bool) -> RuleCondition {
7575
let mut conditions = vec![
7676
RuleCondition::ResourcePathEndsWith(".js".to_string()),
7777
RuleCondition::ResourcePathEndsWith(".jsx".to_string()),
@@ -98,35 +98,28 @@ fn match_js_extension(enable_mdx_rs: bool) -> Vec<RuleCondition> {
9898
.as_mut(),
9999
);
100100
}
101-
conditions
101+
RuleCondition::any(conditions)
102102
}
103103

104104
/// Returns a module rule condition matches to any ecmascript (with mdx if
105105
/// enabled) except url reference type. This is a typical custom rule matching
106106
/// condition for custom ecma specific transforms.
107107
pub(crate) fn module_rule_match_js_no_url(enable_mdx_rs: bool) -> RuleCondition {
108-
let conditions = match_js_extension(enable_mdx_rs);
109-
110108
RuleCondition::all(vec![
111109
RuleCondition::not(RuleCondition::ReferenceType(ReferenceType::Url(
112110
UrlReferenceSubType::Undefined,
113111
))),
114-
RuleCondition::any(conditions),
112+
match_js_extension(enable_mdx_rs),
115113
])
116114
}
117115

118116
pub(crate) fn module_rule_match_pages_page_file(
119117
enable_mdx_rs: bool,
120118
pages_directory: FileSystemPath,
121119
) -> RuleCondition {
122-
let conditions = match_js_extension(enable_mdx_rs);
123-
124120
RuleCondition::all(vec![
125-
RuleCondition::not(RuleCondition::ReferenceType(ReferenceType::Url(
126-
UrlReferenceSubType::Undefined,
127-
))),
121+
module_rule_match_js_no_url(enable_mdx_rs),
128122
RuleCondition::ResourcePathInExactDirectory(pages_directory),
129-
RuleCondition::any(conditions),
130123
])
131124
}
132125

‎crates/next-core/src/next_shared/transforms/next_strip_page_exports.rs

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use swc_core::ecma::ast::Program;
77
use turbo_tasks::ResolvedVc;
88
use turbo_tasks_fs::FileSystemPath;
99
use turbopack::module_options::{ModuleRule, ModuleRuleEffect, RuleCondition};
10+
use turbopack_core::reference_type::{EntryReferenceSubType, ReferenceType};
1011
use turbopack_ecmascript::{CustomTransformer, EcmascriptInputTransform, TransformContext};
1112

1213
use super::module_rule_match_js_no_url;
@@ -16,31 +17,36 @@ pub fn get_next_pages_transforms_rule(
1617
pages_dir: FileSystemPath,
1718
export_filter: ExportFilter,
1819
enable_mdx_rs: bool,
19-
extra_conditions: impl IntoIterator<Item = RuleCondition>,
2020
) -> Result<ModuleRule> {
2121
// Apply the Next SSG transform to all pages.
2222
let strip_transform =
2323
EcmascriptInputTransform::Plugin(ResolvedVc::cell(Box::new(NextJsStripPageExports {
2424
export_filter,
2525
}) as _));
26-
let mut conditions = RuleCondition::all(vec![
27-
RuleCondition::all(vec![
28-
RuleCondition::ResourcePathInExactDirectory(pages_dir.clone()),
29-
RuleCondition::not(RuleCondition::ResourcePathInExactDirectory(
30-
pages_dir.join("api")?,
31-
)),
32-
RuleCondition::not(RuleCondition::any(vec![
33-
// TODO(alexkirsz): Possibly ignore _app as well?
34-
RuleCondition::ResourcePathEquals(pages_dir.join("_document.js")?),
35-
RuleCondition::ResourcePathEquals(pages_dir.join("_document.jsx")?),
36-
RuleCondition::ResourcePathEquals(pages_dir.join("_document.ts")?),
37-
RuleCondition::ResourcePathEquals(pages_dir.join("_document.tsx")?),
38-
])),
39-
]),
26+
let conditions = RuleCondition::all(vec![
27+
RuleCondition::ResourcePathInExactDirectory(pages_dir.clone()),
28+
RuleCondition::not(RuleCondition::ResourcePathInExactDirectory(
29+
pages_dir.join("api")?,
30+
)),
31+
RuleCondition::not(RuleCondition::any(vec![
32+
// TODO(alexkirsz): Possibly ignore _app as well?
33+
RuleCondition::ResourcePathEquals(pages_dir.join("_document.js")?),
34+
RuleCondition::ResourcePathEquals(pages_dir.join("_document.jsx")?),
35+
RuleCondition::ResourcePathEquals(pages_dir.join("_document.ts")?),
36+
RuleCondition::ResourcePathEquals(pages_dir.join("_document.tsx")?),
37+
])),
4038
module_rule_match_js_no_url(enable_mdx_rs),
41-
RuleCondition::all(extra_conditions.into_iter().collect()),
39+
// For PagesData we strip components and for everything else we drop data exports.
40+
match export_filter {
41+
ExportFilter::StripDataExports => RuleCondition::any(vec![
42+
RuleCondition::ReferenceType(ReferenceType::Entry(EntryReferenceSubType::Page)),
43+
RuleCondition::ReferenceType(ReferenceType::Entry(EntryReferenceSubType::PagesApi)),
44+
]),
45+
ExportFilter::StripDefaultExport => {
46+
RuleCondition::ReferenceType(ReferenceType::Entry(EntryReferenceSubType::PageData))
47+
}
48+
},
4249
]);
43-
conditions.flatten();
4450
Ok(ModuleRule::new(
4551
conditions,
4652
vec![ModuleRuleEffect::ExtendEcmascriptTransforms {

‎turbopack/crates/turbopack/src/module_options/module_rule.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ pub struct ModuleRule {
2121

2222
impl ModuleRule {
2323
/// Creates a new module rule. Will not match internal references.
24-
pub fn new(condition: RuleCondition, effects: Vec<ModuleRuleEffect>) -> Self {
24+
pub fn new(mut condition: RuleCondition, effects: Vec<ModuleRuleEffect>) -> Self {
25+
condition.flatten();
2526
ModuleRule {
2627
condition,
2728
effects,
@@ -30,7 +31,8 @@ impl ModuleRule {
3031
}
3132

3233
/// Creates a new module rule. Will only match internal references.
33-
pub fn new_internal(condition: RuleCondition, effects: Vec<ModuleRuleEffect>) -> Self {
34+
pub fn new_internal(mut condition: RuleCondition, effects: Vec<ModuleRuleEffect>) -> Self {
35+
condition.flatten();
3436
ModuleRule {
3537
condition,
3638
effects,
@@ -39,7 +41,8 @@ impl ModuleRule {
3941
}
4042

4143
/// Creates a new module rule. Will match all references.
42-
pub fn new_all(condition: RuleCondition, effects: Vec<ModuleRuleEffect>) -> Self {
44+
pub fn new_all(mut condition: RuleCondition, effects: Vec<ModuleRuleEffect>) -> Self {
45+
condition.flatten();
4346
ModuleRule {
4447
condition,
4548
effects,

0 commit comments

Comments
 (0)