-
Notifications
You must be signed in to change notification settings - Fork 42
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
perf: reduce some clones and pre-allocate some collections #215
Conversation
for root in &self.roots { | ||
validate(root, None, types_only, false, &mut seen, &|s| { | ||
self.try_get(s).map(|o| o.cloned()) |
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 seemed heavy.
src/graph.rs
Outdated
@@ -769,15 +769,16 @@ impl ModuleGraph { | |||
/// returning the "final" module. | |||
pub fn resolve(&self, specifier: &ModuleSpecifier) -> ModuleSpecifier { | |||
let mut redirected_specifier = specifier; | |||
let mut seen = HashSet::new(); | |||
let max_redirects = 6; |
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.
Does this match the number of allowed redirects in CLI file fetcher?
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.
That is 10, I will increase this
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.
LGTM. Thinking about this max redirect count mismatch, I'm wondering if FileFetcher
in Deno should be a crate...
The redirects here are just a module graph thing so the code couldn’t be shared. |
No description provided.