Skip to content

feat: load source phase imports as assets#630

Merged
nayeemrmn merged 14 commits intodenoland:mainfrom
nayeemrmn:source-phase-imports
Dec 8, 2025
Merged

feat: load source phase imports as assets#630
nayeemrmn merged 14 commits intodenoland:mainfrom
nayeemrmn:source-phase-imports

Conversation

@nayeemrmn
Copy link
Collaborator

@nayeemrmn nayeemrmn commented Dec 3, 2025

@nayeemrmn nayeemrmn requested review from bartlomieju, dsherret and nathanwhit and removed request for bartlomieju, dsherret and nathanwhit December 3, 2025 20:05
Copy link
Member

@dsherret dsherret left a comment

Choose a reason for hiding this comment

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

Looks great! A few small comments.

Comment on lines +5085 to +5088
// TODO(nayeemrmn): We need to load the module to validate the actual
// media type for source-phase-import eligibility. Don't treat
// source-phase-imported modules as assets. Get the actual media type.
let inferred_media_type = MediaType::from_specifier(specifier);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We need to download source-only targets so the media type is at least known. Most likely not for this PR though

Copy link
Member

Choose a reason for hiding this comment

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

This is a pretty big todo. We should work on it after this lands.

@bartlomieju bartlomieju requested a review from dsherret December 8, 2025 13:01
Copy link
Member

@dsherret dsherret left a comment

Choose a reason for hiding this comment

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

Nice. LGTM!

Comment on lines +5085 to +5088
// TODO(nayeemrmn): We need to load the module to validate the actual
// media type for source-phase-import eligibility. Don't treat
// source-phase-imported modules as assets. Get the actual media type.
let inferred_media_type = MediaType::from_specifier(specifier);
Copy link
Member

Choose a reason for hiding this comment

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

This is a pretty big todo. We should work on it after this lands.

@nayeemrmn nayeemrmn merged commit 374a95b into denoland:main Dec 8, 2025
17 checks passed
@nayeemrmn nayeemrmn deleted the source-phase-imports branch December 15, 2025 23:36
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