fix(compartment-mapper): invert keys/values of renames in captureFromMap #2667
+306
−392
Conversation
This file contains 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
kriskowal
requested changes
Jan 8, 2025
… makeArchiveCompartmentMap This change isolates the duplicated code in `archive-lite.js` and `capture-lite.js` into a new internal module, `digest.js`. It provides `digestCompartmentMap()`, which is now consumed by both `archive-lite.js` and `capture-lite.js`. Both `makeArchiveCompartmentMap()` and `captureFromMap()` now pass new properties from `digestCompartmentMap()` through to their consumers: `newToOldCompartmentMap` and `oldToNewCompartmentMap`. **The `compartmentRenames` property is now soft-deprecated** (since its naming is ambiguous). The same property is now `newToOldCompartmentMap`; `compartmentRenames` should eventually be removed. _Note_: The duplicated code drifted slightly in `archive-lite.js`; see e3b310d. These changes were kept and put into `digest.js`.
boneskull
force-pushed
the
invert-renames
branch
from
fdacf52 to
aa1ee5f
Compare
|
@kriskowal I've applied the requested changes and updated this PR's description accordingly |
kriskowal
approved these changes
Jan 10, 2025
kriskowal
changed the title
|
I took the liberty of removing the |
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.
This more accurately solves for the use case of
compartmentRenames, because the main use case demands the record is inverted. We can plainly see this issue incaptureFromMap's test suite (which this change simplifies).In a future change, branded types could describe the distinction between key and value, which are currently both
string.This change isolates the duplicated code in
archive-lite.jsandcapture-lite.jsinto a new internal module,digest.js.It provides
digestCompartmentMap(), which is now consumed by botharchive-lite.jsandcapture-lite.js.Both
makeArchiveCompartmentMap()andcaptureFromMap()now pass new properties fromdigestCompartmentMap()through to their consumers:newToOldCompartmentMapandoldToNewCompartmentMap.The
compartmentRenamesproperty is now soft-deprecated (since its naming is ambiguous). The same property is nownewToOldCompartmentMap;compartmentRenamesshould eventually be removed.Note: The duplicated code drifted slightly in
archive-lite.js; see e3b310d. These changes were kept and put intodigest.js.