Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fixed env config renaming #7823

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
+27 −36
Open

fixed env config renaming #7823

Changes from 5 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
dd65bec
fixed env config renaming
NitsanCohen770 Aug 22, 2023
4a585e8
fix if check and env with version
NitsanCohen770 Aug 23, 2023
bb9784c
fix replace env with version on bitmap
NitsanCohen770 Aug 23, 2023
3cc1424
fix rename new component issue
NitsanCohen770 Aug 23, 2023
eddaae8
Merge branch 'master' into fix-bitmap-env-rename
NitsanCohen770 Aug 23, 2023
4c52a07
remove redundant getBitmapEntry
NitsanCohen770 Aug 23, 2023
1017585
Merge branch 'master' into fix-bitmap-env-rename
NitsanCohen770 Aug 23, 2023
3b76652
replace the method with new workspace method
NitsanCohen770 Aug 23, 2023
610fcc4
remove renameAspectInConfig as it is not used now
NitsanCohen770 Aug 23, 2023
0dbb915
remove import
NitsanCohen770 Aug 23, 2023
24fdf83
fix rename new components
NitsanCohen770 Aug 23, 2023
b0b110f
Merge branch 'master' into fix-bitmap-env-rename
NitsanCohen770 Aug 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions scopes/workspace/workspace/bit-map.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,20 +191,32 @@ export class BitMap {
this.legacyBitMap.components.forEach((componentMap) => {
const config = componentMap.config;
if (!config) return;

Object.keys(config).forEach((aspectId) => {
if (aspectId === sourceId.toString()) {
let fullSourceId: string;
try {
fullSourceId = this.getBitmapEntry(sourceId).id.toStringWithoutVersion();
Copy link
Member

@GiladShoham GiladShoham Aug 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Instead of try catch you can use
getBitmapEntryIfExist
And instead of doing toStringWithoutVersion
you can pass a second arg to the get like this:

{
  "ignoreVersion": true
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks
Actually, I realized that the sourceId is already a componentId so this whole get entry is just redundant.

} catch (error) {
// If entry not found it's probably new, keep the sourceId as is
fullSourceId = sourceId.toString();
}

const aspectIdWithoutVersion = ComponentID.fromString(aspectId).toStringWithoutVersion();
if (aspectIdWithoutVersion === fullSourceId) {
config[targetId.toString()] = config[aspectId];
delete config[aspectId];
this.markAsChanged();
}

if (aspectId === EnvsAspect.id) {
const envConfig = config[aspectId];
if (envConfig !== REMOVE_EXTENSION_SPECIAL_SIGN && envConfig.env === sourceId.toString()) {
if (envConfig !== REMOVE_EXTENSION_SPECIAL_SIGN && envConfig.env === fullSourceId) {
Copy link
Member

Choose a reason for hiding this comment

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

The envConfig.env might have a value with a version, in that case, your new check will miss it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This check is for this prop:

 "env": "teambit.react/react-env"
            }

afaik it can't be with the version over there.

I think the issue you are referring to is with the env itself as a prop for example:

"teambit.react/react-env@1.0.6": {},

In that case, this one is resolved here:

  if (aspectIdWithoutVersion === sourceIdWithoutVersion) {

envConfig.env = targetId.toString();
this.markAsChanged();
}
}
});

componentMap.config = config;
});
}
Expand Down