-
Notifications
You must be signed in to change notification settings - Fork 103
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
fix: component set maps treat encoded and decoded keys as the same #1138
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approved with non-blocking comments/suggestions
// Internal map of decoded keys to encoded keys. | ||
// E.g., { 'foo-v1.3 Layout': 'foo-v1%2E3 Layout' } | ||
// This is initialized in the `keysMap` getter. | ||
private internalkeysMap!: Map<string, string>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not init it during declaration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. I originally had the Map initialization done during declaration and it would fail during construction due to the way super
works. super
must be the first call in a constructor, which means the map init was done after that. So if you construct a new map by passing in a nested array, it will call set()
which needs the internal map but it wasn't initialized until later. To solve this problem I used the getter and conditionally initialized it there.
I'll add a comment.
// This is initialized in the `keysMap` getter. | ||
private internalkeysMap!: Map<string, string>; | ||
|
||
private get keysMap(): Map<string, string> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you init at declaration, you also wouldn't have to have a getter for a private prop--you could just use the prop.
if (super.has(decodedKey)) { | ||
return decodedKey; | ||
} | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perf, maybe? you could try checking the keysMap 2nd instead of 3rd to defer the decode operation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It needs to be done either way with the internal Map implementation.
QA (repeating other PR's steps forcedotcom/source-tracking#458 add the title field to the Broker 1.1 layout and save
create new layout for Broker__c named 1.1 año día create new default dir Profiles also use encodingcreate
new pkgdir as default |
What does this PR do?
ComponentSets
useMaps
with keys that are built from metadata fullNames. Some fullNames such as Layouts and Profiles can contain encoded characters since they allow end users to name them with any character. When these metadata types are retrieved from the org the fullNames are encoded, which results in encoded filenames written to disk. However, for source tracking the queries return fullNames that are decoded which results in no local matches when they actually refer to the same metadata.To fix this situation we can search the
ComponentSet
maps using the keys "as is", and if not found try with a decoded or encoded key.What issues does this PR fix or reference?
@W-11658886@
forcedotcom/cli#1683
See this PR in the source tracking library for testing ideas.