refactor: convert site-import directory to TypeScript
#1492
Conversation
| export function getGlyphForStatus( status: StepStatus, runningSprite: RunningSprite ): string { | ||
| export function getGlyphForStatus( | ||
| status: StepStatus | string, |
There was a problem hiding this comment.
Because of this: https://github.com/Automattic/vip-cli/pull/1492/files#diff-e2db69e07a54f1d529171080f7d84e280b1dbb9b2e171ca55dc24e7e4fc571b4R207
We pass something that is not a StepStatus to this function.
src/lib/site-import/status.ts
Outdated
| const [ importJob ] = jobs; | ||
| const importJob = jobs?.[ 0 ] ?? {}; |
There was a problem hiding this comment.
TS complains that jobs could be undefined. In this case, destructuring will fail.
There was a problem hiding this comment.
nit: This change currently affects the logic a little. Maybe
const [ importJob ] = jobs ?? [];instead?
There was a problem hiding this comment.
I was checking which line might care about this change - looks like this line:
vip-cli/src/lib/site-import/status.ts
Line 279 in b149b4d
| statusMessage = `Success ${ sprite } imported data should be visible on your site ${ env.primaryDomain.name }.`; | ||
| statusMessage = `Success ${ sprite } imported data should be visible on your site ${ | ||
| env.primaryDomain?.name ?? 'N/A' | ||
| }.`; |
There was a problem hiding this comment.
These things are to keep ESLint happy: our ruleset does not alow undefined as a template parameter.
| status = await getStatus( api, app.id, env.id ); | ||
| status = await getStatus( api, app.id ?? -1, env.id ?? -1 ); |
There was a problem hiding this comment.
In theory, according to GQL definitions, id could be null or undefined. I chose -1 to make the request fail.
| .filter( Boolean ) as number[]; | ||
| importJob.completedAt = new Date( Math.max( ...timestamps, 0 ) * 1000 ).toUTCString(); |
There was a problem hiding this comment.
Math.max(), according to TS, does not like all kinds of null and undefined.
| @@ -351,12 +384,11 @@ ${ maybeExitPrompt } | |||
| commandOutput: failedImportStep.output, | |||
| error: 'Import step failed', | |||
| stepName: failedImportStep.name, | |||
| errorText: failedImportStep.error, | |||
There was a problem hiding this comment.
I had to drop errorText:
- It is not used in
getErrorMessage() AppEnvironmentStatusProgressStepdoes not have theerrorfield :-(
| progressTracker.setStepsFromServer( jobSteps ); | ||
| progressTracker.setStepsFromServer( jobSteps as unknown as StepFromServer[] ); |
There was a problem hiding this comment.
I am not very proud of this :-(
src/lib/site-import/status.ts
Outdated
| const [ importJob ] = jobs; | ||
| const importJob = jobs?.[ 0 ] ?? {}; |
There was a problem hiding this comment.
nit: This change currently affects the logic a little. Maybe
const [ importJob ] = jobs ?? [];instead?
src/lib/site-import/status.ts
Outdated
| const [ importJob ] = jobs; | ||
| const importJob = jobs?.[ 0 ] ?? {}; |
There was a problem hiding this comment.
I was checking which line might care about this change - looks like this line:
vip-cli/src/lib/site-import/status.ts
Line 279 in b149b4d
src/lib/site-import/status.ts
Outdated
| inImportProgress: boolean; | ||
| commandOutput: string[] | null; | ||
| error: string; | ||
| stepName: string; |
There was a problem hiding this comment.
nit: We could use StepName | string where StepName is an enum consisting of import_preflights, importing_db, ...etc
| @@ -370,7 +402,9 @@ ${ maybeExitPrompt } | |||
|
|
|||
| overallStatus = 'running'; | |||
|
|
|||
| setTimeout( checkStatus, IMPORT_SQL_PROGRESS_POLL_INTERVAL ); // NOSONAR | |||
| setTimeout( () => { | |||
| void checkStatus(); | |||
There was a problem hiding this comment.
TIL of the void operator.
sjinks
force-pushed
the
convert/site-import
branch
from
b8f27c6
to
4d4c7a4
Compare
Description
This PR converts files from the
src/lib/site-importdirectory to TypeScript.Steps to Test
CI should pass.