chore: convert init command to TS #708
Conversation
gvarandas
requested review from
Esemesek,
grabbou and
thymikee
as code owners
| @@ -22,7 +19,7 @@ afterEach(() => { | |||
| }); | |||
|
|
|||
| test('installTemplatePackage', async () => { | |||
| jest.spyOn(PackageManger, 'install').mockImplementationOnce(() => {}); | |||
| jest.spyOn(PackageManger, 'install').mockImplementationOnce(() => null); | |||
There was a problem hiding this comment.
Changed the mock implementation to avoid type conflicts. There was no change in the overall outcome from the unit tests since we're not asserting the result from the mock.
| @@ -63,7 +60,7 @@ test('copyTemplate', async () => { | |||
| const CWD = '.'; | |||
|
|
|||
| jest.spyOn(path, 'resolve').mockImplementationOnce((...e) => e.join('/')); | |||
| jest.spyOn(copyFiles, 'default').mockImplementationOnce(() => {}); | |||
| jest.spyOn(copyFiles, 'default').mockImplementationOnce(() => null); | |||
| ])("'%s' is invalid name", ({name, error}: {name: string, error: Error}) => { | ||
| // @ts-ignore-next-line FIXME extending the Error class causes weird TS validation errors | ||
| // https://stackoverflow.com/questions/41102060/typescript-extending-error-class | ||
| ])("'%s' is invalid name", ({name, error}: {name: string; error: Error}) => { |
There was a problem hiding this comment.
This is the only unsolved case from the conversion.
Apparently TS has weird issues when extending the Error class by not recognizing the correct properties.
Tried different solutions with no success. 😞
| @@ -103,7 +103,7 @@ async function installPods({ | |||
| }: { | |||
| projectName: string; | |||
| loader?: ora.Ora; | |||
| shouldUpdatePods: boolean; | |||
| shouldUpdatePods?: boolean; | |||
There was a problem hiding this comment.
Updated the type definition for this property as a proposition since we're checking if it is passed or not in the method execution.
I'll be happy to revert this and explicitly pass the false option on commands/init/initCompat:88 if needed.
gvarandas
force-pushed
the
chore/init-ts-migration
branch
from
147cbc3 to
849c1c2
Compare
thymikee
force-pushed
the
chore/init-ts-migration
branch
from
849c1c2 to
10a45f5
Compare
| @@ -136,7 +140,7 @@ async function createFromTemplate({ | |||
| projectName, | |||
| projectTitle, | |||
| placeholderName: templateConfig.placeholderName, | |||
| titlePlaceholder: templateConfig.titlePlaceholder, | |||
| placeholderTitle: templateConfig.titlePlaceholder, | |||
There was a problem hiding this comment.
Good catch. cc @Esemesek – 0.61 is right behind the corner, so I guess we'll need to keep this name for a while?
Summary:
Part of issue #683
Converted
commands/init/*to typescript.The conversion involved the following steps:
.jsto.ts;@flowannotations from the header;Test Plan:
yarn flowandyarn lintto guarantee static analysis with no errorsyarn testto guarantee unit test coverage (this also involved making sure that the new snapshots are generated in the correct.tsformat).cli, following the guidelines from theCONTRIBUTING.mdfile.