-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
add rules to alter resolve/module context depending on context path #3108
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
5 Ignored Deployments
|
🟢 CI successful 🟢Thanks |
This comment was marked as outdated.
This comment was marked as outdated.
Benchmark for 706125dClick to view benchmark
|
rules: vec![( | ||
ContextCondition::InDirectory("node_modules".to_string()), | ||
Some( | ||
ResolveOptionsContext { | ||
enable_node_modules: true, | ||
custom_conditions: vec!["development".to_string()], | ||
import_map: Some(next_client_import_map), | ||
fallback_import_map: Some(next_client_fallback_import_map), | ||
resolved_map: Some(next_client_resolved_map), | ||
browser: true, | ||
module: true, | ||
..Default::default() | ||
} | ||
.cell(), | ||
), | ||
)], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be:
let shared_options = ResolveOptionsContext {
enable_node_modules: true,
custom_conditions: vec!["development".to_string()],
import_map: Some(next_client_import_map),
fallback_import_map: Some(next_client_fallback_import_map),
resolved_map: Some(next_client_resolved_map),
browser: true,
module: true,
..Default::default()
};
Then spread that into both this and the one above. This way, if we ever add new keys in the future, they will be applied to both correctly.
The same remark applies to all of the below.
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add comments to the public items.
if let &Some(new_context) = new_context { | ||
return Ok(ModuleOptionsVc::new(path, new_context)); | ||
} else { | ||
break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we ever run into that case? I think we only have Some(
above, maybe it shouldn't be an option?
Benchmark for e9099ad
Click to view full benchmark
|
Fixes WEB-43
Closes #3104