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

feat(compile): support discovering modules for more dynamic arguments #21381

Merged
merged 12 commits into from
Dec 1, 2023

Conversation

dsherret
Copy link
Member

@dsherret dsherret commented Nov 29, 2023

This PR causes Deno to include more files in the graph based on how a template literal looks that's provided to a dynamic import:

const file = await import(`./dir/${expr}`);

In this case, it will search the dir directory and descendant directories for any .js/jsx/etc modules and include them in the graph.

To opt out of this behaviour, move the template literal to a separate line:

const specifier = `./dir/${expr}`
const file = await import(specifier);

Also, closes (due to deno_doc upgrade):

Closes #21405
Closes #21403
Closes #21390

cli/util/import_map.rs Outdated Show resolved Hide resolved
@dsherret dsherret marked this pull request as ready for review December 1, 2023 15:01
Comment on lines 96 to 106
if !success {
log::warn!(
"{} Dynamic import was not analyzable and won't use the import map once published.\n at {}",
crate::colors::yellow("Warning"),
format_range_with_colors(&deno_graph::Range {
specifier: url.clone(),
start: dep.range.start.clone(),
end: dep.range.end.clone(),
})
);
}
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a test that exercised this warning?

Copy link
Member Author

@dsherret dsherret Dec 1, 2023

Choose a reason for hiding this comment

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

I wanted to, but we don't have any integration tests for publishing at the moment and refactoring this out to a returned diagnostic didn't seem worth it since there are some test cases for this scenario where it doesn't replace it.

@dsherret dsherret enabled auto-merge (squash) December 1, 2023 17:37
@dsherret dsherret merged commit a1d823e into denoland:main Dec 1, 2023
14 checks passed
@iuioiua
Copy link
Collaborator

iuioiua commented Dec 1, 2023

Awesome. The deno_doc upgrade is going to help us document the Standard Library better. Thanks for this!

@dsherret dsherret deleted the feat_dynamic_arg_expr branch December 1, 2023 20:18
@dsherret
Copy link
Member Author

dsherret commented Dec 1, 2023

@iuioiua and thank you for trying out the --lint flag and pointing out issues!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants