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

Document and test workflow for aliasing external modules #3760

Merged
merged 4 commits into from
Oct 17, 2024

Conversation

lihaoyi
Copy link
Member

@lihaoyi lihaoyi commented Oct 17, 2024

Fixes #299

I'm a bit surprised that this just works, but it does!

The module's millSourcePath and out/ folder path remain at their original locations as the mill.define.Ctx is still provided by the top-level ExternalModule constructor, it's just during resolution the added method def allows it to get picked up during object graph traversal

@lihaoyi lihaoyi requested review from lefou and lolgab October 17, 2024 07:27
@lefou
Copy link
Member

lefou commented Oct 17, 2024

@lihaoyi
Copy link
Member Author

lihaoyi commented Oct 17, 2024

I think #3715 is kind of adjacent, since it's more about preventing the stackoverflow. Whether the referenced module ends up discoverable or not isn't the main issue raised in that ticket, although it's certainly worth discussing.

One potential issue with def aliases is that one module can now have different selectors turn up when you resolve _. For external modules that's not a big deal since the external module namespace and internal module namespace are kept separate, but once you start aliasing internal modules then the duplication occurs. Even in scenarios without infinite loops, when someone does a mill resolve __ we probably don't want to "expand out" every module alias and print a huge wide tree when it may actually just be a relatively narrow DAG

@lihaoyi lihaoyi merged commit 2e7e281 into com-lihaoyi:main Oct 17, 2024
24 checks passed
@lefou lefou added this to the 0.12.0 milestone Oct 17, 2024
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.

Add ability to alias Modules (specially external modules)
2 participants