LG-7291 TypeScript refactoring for 6777#6769
Conversation
-- What With the addition of new checks for `maxAttemptsBeforeNativeCamera`, which triggers the use of the native camera after a certain number of failed Acuant attempts, we needed to update some of the type definitions. In addition to doing so, we have converted several of the affected files to full TypeScript (from JSDoc annotations).
changelog: Improvements, TypeScript, rebasing typescript changes on main and associated fixes
changelog: Improvements, TypeScript, fixing linting issues
app/javascript/packages/document-capture/components/acuant-capture.tsx
Outdated
Show resolved
Hide resolved
app/javascript/packages/document-capture/components/acuant-capture.tsx
Outdated
Show resolved
Hide resolved
app/javascript/packages/document-capture/components/acuant-capture.tsx
Outdated
Show resolved
Hide resolved
app/javascript/packages/document-capture/components/acuant-capture.tsx
Outdated
Show resolved
Hide resolved
| /** | ||
| * Image width, or null if unknown | ||
| */ | ||
| width?: number | null; |
There was a problem hiding this comment.
Before, these were number | null, but not optional.
| width?: number | null; | |
| width: number | null; |
There was a problem hiding this comment.
Thanks, I flipped the original non-optionals back to being non-optional in that interface 8d31f0a
…ture.tsx Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
…ture.tsx Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
…ture.tsx Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
…ture.tsx Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
Also switching to the global window object, which already is annotated with the AcuantJavaScriptWebSdk interface changelog: Improvements, TypeScript, updating interface property types
aduth
left a comment
aduth
left a comment
There was a problem hiding this comment.
Couple minor suggestions, but looks great overall!
| /** | ||
| * @typedef {ImageAnalyticsPayload & _AcuantImageAnalyticsPayload} AcuantImageAnalyticsPayload | ||
| */ | ||
| type AcuantImageAnalyticsPayload = ImageAnalyticsPayload & _AcuantImageAnalyticsPayload; |
There was a problem hiding this comment.
This is a good one-to-one port, but I believe it was implemented the way it was previously due to limitations of a good "extends" mechanism which would be available to us in TypeScript proper, i.e.
interface AcuantImageAnalyticsPayload extends ImageAnalyticsPayload {
documentType: AcuantDocumentTypeLabel;
// ...
}What do you think about making that revision?
app/javascript/packages/document-capture/components/acuant-capture.tsx
Outdated
Show resolved
Hide resolved
app/javascript/packages/document-capture/components/acuant-capture.tsx
Outdated
Show resolved
Hide resolved
app/javascript/packages/document-capture/components/acuant-capture.tsx
Outdated
Show resolved
Hide resolved
app/javascript/packages/document-capture/components/acuant-capture.tsx
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,102 @@ | |||
| import { createContext, useState } from 'react'; | |||
There was a problem hiding this comment.
Is this file meant to be included?
There was a problem hiding this comment.
It is, sort of, but I made the jsx -> tsx file rename more explicit now in 6d0a377
…ture.tsx Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
…ture.tsx Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
…ture.tsx Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
…ture.tsx Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
Additionally, I have renamed failed-capture-context.jsx to tsx explicitly changelog: Improvements, TypeScript, updating type definitions
changelog: Improvements, TypeScript, removing old type definition
aduth
left a comment
aduth
left a comment
There was a problem hiding this comment.
LGTM! Love to see more TypeScript.
| isAssessedAsBlurry: boolean; | ||
| } | ||
|
|
||
| interface FailedCaptureAttemptsContextInterface { |
There was a problem hiding this comment.
I don't have a strong stance either way, and I know there are some conventions incorporating "Interface" in the name, but elsewhere I'd been tending to suffix "Value" for the interface describing the context value shape. I think we should aim to be consistent whichever way we want to go with the naming.
3 participants
What
This PR corresponds to LG-7291.
In the course of making changes for LG-6777 (this PR), we decided to pull out the changes related to TypeScript refactoring and wait until the origin PR had been merged.
These changes refactor all filed affected by LG-6777 from JSDoc type annotations to full TypeScript.