-
Notifications
You must be signed in to change notification settings - Fork 4
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!: move from isWarning and isError to status prop #1973
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,156 @@ | ||
import { dedent } from 'ts-dedent'; | ||
|
||
import { updateStatusProp } from './alpha14-to-alpha15'; | ||
import { createTestSourceFile } from '../helpers'; | ||
|
||
describe('alpha14-to-alpha15', () => { | ||
it('updates an appropriate isWarning prop', () => { | ||
const sourceFileText = dedent` | ||
import {FieldNote} from '@chanzuckerberg/eds'; | ||
|
||
export default function Component() { | ||
return ( | ||
<FieldNote isWarning /> | ||
) | ||
} | ||
`; | ||
|
||
const sourceFile = createTestSourceFile(sourceFileText); | ||
|
||
updateStatusProp({ | ||
file: sourceFile, | ||
}); | ||
|
||
expect(sourceFile.getText()).toEqual(dedent` | ||
import {FieldNote} from '@chanzuckerberg/eds'; | ||
|
||
export default function Component() { | ||
return ( | ||
<FieldNote status="warning" /> | ||
) | ||
} | ||
`); | ||
}); | ||
|
||
it('updates an appropriate isError prop', () => { | ||
const sourceFileText = dedent` | ||
import {FieldNote} from '@chanzuckerberg/eds'; | ||
|
||
export default function Component() { | ||
return ( | ||
<FieldNote isError /> | ||
) | ||
} | ||
`; | ||
|
||
const sourceFile = createTestSourceFile(sourceFileText); | ||
|
||
updateStatusProp({ | ||
file: sourceFile, | ||
}); | ||
|
||
expect(sourceFile.getText()).toEqual(dedent` | ||
import {FieldNote} from '@chanzuckerberg/eds'; | ||
|
||
export default function Component() { | ||
return ( | ||
<FieldNote status="critical" /> | ||
) | ||
} | ||
`); | ||
}); | ||
|
||
it('does not update an isError prop on non-EDS component', () => { | ||
const sourceFileText = dedent` | ||
import {FieldNote} from 'somewhere'; | ||
|
||
export default function Component() { | ||
return ( | ||
<FieldNote isError /> | ||
) | ||
} | ||
`; | ||
|
||
const sourceFile = createTestSourceFile(sourceFileText); | ||
|
||
updateStatusProp({ | ||
file: sourceFile, | ||
}); | ||
|
||
expect(sourceFile.getText()).toEqual(dedent` | ||
import {FieldNote} from 'somewhere'; | ||
|
||
export default function Component() { | ||
return ( | ||
<FieldNote isError /> | ||
) | ||
} | ||
`); | ||
}); | ||
|
||
it('updates appropriately when both isError and isWarning prop exists', () => { | ||
const sourceFileText = dedent` | ||
import {FieldNote} from '@chanzuckerberg/eds'; | ||
|
||
export default function Component() { | ||
return ( | ||
<FieldNote isWarning isError /> | ||
) | ||
} | ||
`; | ||
|
||
const sourceFile = createTestSourceFile(sourceFileText); | ||
|
||
updateStatusProp({ | ||
file: sourceFile, | ||
}); | ||
|
||
expect(sourceFile.getText()).toEqual(dedent` | ||
import {FieldNote} from '@chanzuckerberg/eds'; | ||
|
||
export default function Component() { | ||
return ( | ||
<FieldNote status="warning" status="critical" /> | ||
) | ||
} | ||
`); | ||
}); | ||
|
||
it('converts on all component types', () => { | ||
const sourceFileText = dedent` | ||
import {FieldNote, InputField, Select, TextareaField} from '@chanzuckerberg/eds'; | ||
|
||
export default function Component() { | ||
return ( | ||
<div> | ||
<InputField isWarning /> | ||
<FieldNote isWarning></FieldNote> | ||
<Select isError></Select> | ||
<TextareaField isError isWarning></Textarea> | ||
</div> | ||
); | ||
} | ||
`; | ||
|
||
const sourceFile = createTestSourceFile(sourceFileText); | ||
|
||
updateStatusProp({ | ||
file: sourceFile, | ||
}); | ||
|
||
expect(sourceFile.getText()).toEqual(dedent` | ||
import {FieldNote, InputField, Select, TextareaField} from '@chanzuckerberg/eds'; | ||
|
||
export default function Component() { | ||
return ( | ||
<div> | ||
<InputField status="warning" /> | ||
<FieldNote status="warning"></FieldNote> | ||
<Select status="critical"></Select> | ||
<TextareaField status="warning" status="critical"></Textarea> | ||
</div> | ||
); | ||
} | ||
`); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,98 @@ | ||
import { | ||
type JsxAttribute, | ||
SyntaxKind, | ||
type Project, | ||
type SourceFile, | ||
} from 'ts-morph'; | ||
import { isDesignSystemImport } from '../helpers'; | ||
|
||
export default function migration(project: Project) { | ||
const files = project.getSourceFiles(); | ||
const sourceFiles = files.filter((file) => !file.isDeclarationFile()); | ||
|
||
sourceFiles.forEach((sourceFile) => { | ||
updateStatusProp({ file: sourceFile }); // https://github.com/chanzuckerberg/edu-design-system/pull/1973 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jeremiah-clothier FYI For this one-off, custom migration, i've put this into one migration file for now. We shouldn't need this particular one as a common case. The pattern in this case would be that any X-to-Y.ts file would re-use the common cases, and include the one-off transforms there as well to avoid confusion about longevity. |
||
}); | ||
} | ||
|
||
type TransformOptions = { | ||
file: SourceFile; | ||
}; | ||
|
||
const statusComponents = [ | ||
'FieldNote', | ||
'InputField', | ||
'Select', | ||
'TextareaField', | ||
].map((name) => name.toLowerCase()); | ||
|
||
export function updateStatusProp({ file }: TransformOptions) { | ||
// Filter down to the design-system-only imports | ||
const importDeclarations = file | ||
.getImportDeclarations() | ||
.filter( | ||
(importDeclaration) => | ||
!importDeclaration.isTypeOnly() && | ||
isDesignSystemImport(importDeclaration), | ||
); | ||
|
||
const jsxElements = file.getDescendantsOfKind(SyntaxKind.JsxOpeningElement); | ||
const jsxSelfClosingElements = file.getDescendantsOfKind( | ||
SyntaxKind.JsxSelfClosingElement, | ||
); | ||
|
||
// Get the component usages in the given file (which should only work on EDS imports) | ||
[...jsxElements, ...jsxSelfClosingElements].forEach((element) => { | ||
const tagName = element.getTagNameNode().getText(); | ||
const edsTags: string[] = []; | ||
importDeclarations.forEach((importDeclaration) => { | ||
importDeclaration.getNamedImports().forEach((namedImport) => { | ||
edsTags.push(namedImport.getName()); | ||
}); | ||
}); | ||
|
||
if ( | ||
statusComponents.includes(tagName.toLowerCase()) && | ||
edsTags.includes(tagName) | ||
) { | ||
// detect if isWarning exists (at all or with value true) | ||
if ( | ||
isBooleanTrue( | ||
element.getAttribute('isWarning')?.asKind(SyntaxKind.JsxAttribute), | ||
) | ||
) { | ||
element.getAttribute('isWarning')?.remove(); | ||
element.addAttribute({ | ||
name: 'status', | ||
initializer: `"warning"`, | ||
}); | ||
} | ||
|
||
// detect if isError exists (at all or with value true) | ||
if ( | ||
isBooleanTrue( | ||
element.getAttribute('isError')?.asKind(SyntaxKind.JsxAttribute), | ||
) | ||
) { | ||
element.getAttribute('isError')?.remove(); | ||
element.addAttribute({ | ||
name: 'status', | ||
initializer: `"critical"`, | ||
}); | ||
} | ||
} | ||
}); | ||
} | ||
|
||
/** | ||
* Determine whether the attribute evaluates to being set to true | ||
* | ||
* @param attribute the attribute retrieved from the element node | ||
* @returns whether the elements attribute evaluates to true in JSX (exists or exists AND is true) | ||
*/ | ||
function isBooleanTrue(attribute: JsxAttribute | undefined): boolean { | ||
return ( | ||
(attribute && typeof attribute?.getInitializer() === 'undefined') || | ||
attribute?.getInitializer()?.getText() === '{true}' | ||
); | ||
} |
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.
Look great! We should use some consistent naming for the migration files that way
npx eds-migrate --list
returns a list of sorted migrations. Either naming convention is fine, will let you choose@booc0mtaco
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.
@jeremiah-clothier agreed. i'll probably delete this alpha14-to-alpha15 file and test once v15 is released. After all this, there'd only be {old}-to-{new} migrations and the list will be neatly and numerically organized :D
If we do this again, might include the full version suffix for completeness. e.g., 15.alphaX-to-15.alphaY or similar