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

fix(cli): ModuleGraph2 properly handles redirects #7981

Merged
merged 4 commits into from
Oct 15, 2020

Conversation

kitsonk
Copy link
Contributor

@kitsonk kitsonk commented Oct 15, 2020

Fixes issues where both --no-check and deno info do not currently handle redirects.

@kitsonk
Copy link
Contributor Author

kitsonk commented Oct 15, 2020

Bit more detail about this...

ModuleGraph2 no has a hash of all the redirects and when a module is attempted to be resolved, it will keep recursively looking up any redirects until the point where it no longer gets redirected, and then attempts to look the module up in the graph.

Currently the file_fetcher simply returns the "final" Url for a fetched file, so when the module is added to the graph, it only has one potential redirect. If in the future we surface up the entire change, that will "just work" with the graph, because of the recursive way it resolves modules.

I've tried this on my run/check migration branch and it seems to fix all the cases we have tests for.

Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

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

LGTM

cli/file_fetcher.rs Show resolved Hide resolved
cli/module_graph2.rs Outdated Show resolved Hide resolved
cli/module_graph2.rs Outdated Show resolved Hide resolved
@kitsonk kitsonk merged commit bbe4474 into denoland:master Oct 15, 2020
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.

2 participants