Skip to content

feat: ensure source maps are cached#628

Merged
fraidev merged 2 commits intodenoland:mainfrom
fraidev:native_sourcemaps
Nov 26, 2025
Merged

feat: ensure source maps are cached#628
fraidev merged 2 commits intodenoland:mainfrom
fraidev:native_sourcemaps

Conversation

@fraidev
Copy link
Copy Markdown
Contributor

@fraidev fraidev commented Nov 20, 2025

This is needed for: denoland/deno#31268

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Nov 20, 2025

CLA assistant check
All committers have signed the CLA.

@fraidev fraidev changed the title wip chore: ensure source maps are cached Nov 20, 2025
@fraidev fraidev force-pushed the native_sourcemaps branch 6 times, most recently from bc0af36 to 1e21984 Compare November 24, 2025 12:00
@fraidev fraidev marked this pull request as ready for review November 24, 2025 12:04
@dsherret dsherret changed the title chore: ensure source maps are cached feat: ensure source maps are cached Nov 24, 2025
Copy link
Copy Markdown
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.

This looks good from my side, but I'll defer for David to approve

Copy link
Copy Markdown
Contributor

@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! Looks good to me once a test is added for the serialization/deserialization.

pub jsdoc_imports: Vec<JsDocImportInfo>,
/// Source map URL extracted from sourceMappingURL comment
#[serde(skip_serializing_if = "Option::is_none", default)]
pub source_map_url: Option<SpecifierWithRange>,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you add a serialization/deserialization test for this? We need to ensure it's somewhat stable.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure, I added a test in my last commit

@fraidev fraidev merged commit 8701dcc into denoland:main Nov 26, 2025
17 checks passed
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.

4 participants