feat(core): add ModuleId to ResolvedTypeId
#5809
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CodSpeed Performance ReportMerging #5809 will not alter performanceComparing Summary
|
ematipico
approved these changes
Apr 29, 2025
| /// Only the two least significant bits may be set in order to let the type | ||
| /// fit into `ResolvedTypeId`. If more bits become necessary, we may need to | ||
| /// rebalance the layout of `ResolvedTypeId`. | ||
| pub const fn from_u2(bits: u32) -> Self { |
There was a problem hiding this comment.
I actually named it like that on purpose, to make people conscious of the size restraint, even if we don't have a real u2 type :)
crates/biome_module_graph/src/js_module_info/ad_hoc_scope_resolver.rs
Outdated
Show resolved
Hide resolved
arendjr
commented
Apr 29, 2025
crates/biome_module_graph/src/js_module_info/ad_hoc_scope_resolver.rs
Outdated
Show resolved
Hide resolved
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
I'm trying to avoid another huge PR, so I'm taking some smaller steps here :)
The main change in this PR is that
ResolvedTypeIdcan now contain aModuleIdin addition to aTypeId. Such aModuleIdwould only be used in combination withTypeResolverLevel::Module, but with a catch: The types stored in the module graph would not contain these module IDs, because it brings all kinds of complexity if a module needs to know its own ID. So the idea for now is that theAdHocTypeResolverwill attach these module IDs at resolution time in order to keep track of which modules given types came from. I'm not yet sure it will be as easy as this, but that's the direction I'm going in.Other changes are:
JsResolvedPathandJsImportSymbolhave been moved frombiome_module_graphtobiome_js_type_info. This allows us to store the information from where to resolve a given type reference directly intoTypeReferenceitself. TheJsModuleInfoCollectornow convertsResolvedTypeIds ofTypeResolverLevel::Projectinto suchTypeReference::Importedvariants.ResolvedTypeIdwithTypeResolverLevel::Projectas binding ID. It feels a little bit hacky, but it still feels better than introducing anotherTypeDatavariant for such a temporary purpose, because then it would leak into the rest of the type system.biome_module_graphnow has its ownBindingIdinstead of using theBindingIdfrombiome_js_semantic. ThisBindingIdhas aFrom<TypeId>implementation, and vice versa.biome_module_graphthat we weren't actually using.Test Plan
CI should remain green.