-
Notifications
You must be signed in to change notification settings - Fork 19
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
feat(FormFiledGroupHeaderTitleTextObject): handle export specifier #625
feat(FormFiledGroupHeaderTitleTextObject): handle export specifier #625
Conversation
if (!importHasAlias && node.local.name === previousName) { | ||
callContextReport(node, node.local); | ||
} |
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.
Couple more things we should do for this:
- The export shouldn't be reliant on the import not being aliased, so we should remove the !importHasAlias` portion of this conditional.
- We only want to make this update if the export is from Patternfly, so we'd need to update line 9 to destructure both the imports and exports, then we want to only make an update in this ExportSpecifier block if an
interfaceExport
exists.
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.
I misunderstood how the exporting can work. I thought it is necessary to first import the name to be able to reexport it, so the codemod only covers a case like this:
import { FormFiledGroupHeaderTitleTextObject } from "@patternfly/react-core";
export { FormFiledGroupHeaderTitleTextObject as HeaderTitleObject };
If I understand your review correctly, we want to cover this:
export { FormFiledGroupHeaderTitleTextObject as HeaderTitleObject } from "@patternfly/react-core";
But shouldn't we still want to have the case above covered by the codemod?
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.
Ah good point. Yeah we should probably cover both cases then.
code: `import { FormFiledGroupHeaderTitleTextObject } from '@patternfly/react-core'; export { FormFiledGroupHeaderTitleTextObject as TitleTextObject }`, | ||
output: `import { FormFieldGroupHeaderTitleTextObject } from '@patternfly/react-core'; export { FormFieldGroupHeaderTitleTextObject as TitleTextObject }`, |
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.
Related to above, for the tests we want the invalid case to be an export from @patternfly....
, while a valid case would actually be an export like you have here (not from patternfly)
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.
So if I understand, just this:
export { FormFiledGroupHeaderTitleTextObject as TitleTextObject }
should be valid.
But when the import is included:
import { FormFiledGroupHeaderTitleTextObject } from "@patternfly/react-core";
export { FormFiledGroupHeaderTitleTextObject as TitleTextObject };
shouldn't that be invalid? Because if the unaliased import is present, then we know the exported name is 100% the same name we import from Patternfly.
Closes #623