Skip to content

Remove Lint Exceptions: Prop Type Exceptions #10521

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.

Sign up for GitHub

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

Closed
charleyf wants to merge 15 commits into from
Closed

Remove Lint Exceptions: Prop Type Exceptions #10521

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
c40081f
Validate args in in-person-locations spec
charleyf Apr 29, 2024
523a1f3
Convert spinner-dots to .tsx to fix type linting
charleyf Apr 29, 2024
632569b
Add formatHtml type for prop validation
charleyf Apr 29, 2024
f7a5889
Convert selfie canvas to tsx and fix typescript linting
charleyf Apr 29, 2024
30a756f
Change eslint settings
charleyf Apr 29, 2024
a49a6ea
Revert changes to acuant-capture types
charleyf Apr 29, 2024
8576281
Add prop types to in pereson full address component
charleyf Apr 29, 2024
a14e0e0
Add props validation for in person steps
charleyf Apr 29, 2024
9cf2f10
Continue removing fixed files from lint exceptions
charleyf Apr 29, 2024
4f92201
Remove typescript exception for converted file
charleyf Apr 30, 2024
faf2bfe
Fix test issues from new types
charleyf Apr 30, 2024
015e03b
changelog: Internal, IdV, Small refactors to remove linting exceptions
charleyf Apr 30, 2024
4c03d47
Ignore variables with _ rather than lint exception
charleyf May 1, 2024
20c0596
Inline the two default functions
charleyf May 1, 2024
28e7e98
Merge branch 'main' into charley/remove-react-lint-prop-type-exceptions
charleyf May 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions .eslintrc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,16 +85,9 @@
},
{
"files": [
"app/javascript/packages/address-search/components/in-person-locations.spec.tsx",
"app/javascript/packages/components/spinner-dots.jsx",
"app/javascript/packages/document-capture/components/acuant-capture.tsx",
"app/javascript/packages/document-capture/components/acuant-selfie-capture-canvas.jsx",
"app/javascript/packages/document-capture/components/document-side-acuant-capture.jsx",
Comment thread
aduth marked this conversation as resolved.
"app/javascript/packages/document-capture/components/file-image.jsx",
"app/javascript/packages/document-capture/components/file-input.tsx",
"app/javascript/packages/document-capture/components/in-person-location-full-address-entry-post-office-search-step.tsx",
"app/javascript/packages/document-capture/components/in-person-location-post-office-search-step.tsx",
"app/javascript/packages/document-capture/components/in-person-prepare-step.tsx",
"app/javascript/packages/document-capture/components/submission-interstitial.jsx",
"app/javascript/packages/document-capture/components/submission.jsx",
"spec/javascript/packages/document-capture/context/failed-capture-attempts-spec.jsx",
Expand Down
2 changes: 1 addition & 1 deletion app/javascript/packages/address-search/components/in-person-locations.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import sinon from 'sinon';
import type { FormattedLocation } from './in-person-locations';
import InPersonLocations from './in-person-locations';

function NoLocationsViewMock({ address }) {
function NoLocationsViewMock({ address }: { address: string }) {
return (
<div data-testid="no-results-found">
<p>No PO found</p>
Expand Down
15 changes: 7 additions & 8 deletions ...ript/packages/components/spinner-dots.jsx → ...ript/packages/components/spinner-dots.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
/**
* @typedef SpinnerDotsProps
*
* @prop {boolean=} isCentered Whether to absolutely-position the element at its container's center.
* Defaults to false.
* @prop {string=} className Optional class name.
*/
interface SpinnerDotsProps {
// Optional whether to absolutely-position the element at its container's center. Defaults to false.
isCentered?: boolean;
// Optional class name.
className?: string;
}

/**
* @param {SpinnerDotsProps} props
*/
function SpinnerDots({ isCentered, className }) {
function SpinnerDots({ isCentered, className }: SpinnerDotsProps) {
const classes = ['spinner-dots', isCentered && 'spinner-dots--centered', className]
.filter(Boolean)
.join(' ');
Expand Down
10 changes: 9 additions & 1 deletion ...mponents/acuant-selfie-capture-canvas.jsx → ...mponents/acuant-selfie-capture-canvas.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,15 @@ function LoadingSpinner() {
);
}

function AcuantSelfieCaptureCanvas({ imageCaptureText, onSelfieCaptureClosed }) {
interface AcuantSelfieCaptureCanvasProps {
imageCaptureText: string;
onSelfieCaptureClosed: () => void;
}

function AcuantSelfieCaptureCanvas({
imageCaptureText,
onSelfieCaptureClosed,
}: AcuantSelfieCaptureCanvasProps) {
const { isReady } = useContext(AcuantContext);
const { t } = useI18n();
// The Acuant SDK script AcuantPassiveLiveness attaches to whatever element has
Expand Down
4 changes: 3 additions & 1 deletion ...capture/components/in-person-location-full-address-entry-post-office-search-step.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,12 @@ const USPS_RESPONSE = [
},
];

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const defaultRegisterField = (string) => undefined;
const DEFAULT_PROPS = {
toPreviousStep() {},
onChange() {},
registerField() {},
registerField: defaultRegisterField,

@aduth aduth May 1, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we instead update registerField to support void as a supported return type?

identity-idp/app/javascript/packages/form-steps/form-steps.tsx

Line 35 in a5c23dc

) => undefined | RefCallback<HTMLElement>;

➡️

) => undefined | void | RefCallback<HTMLElement>;

@charleyf charleyf May 1, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the inline exceptions, but adding void didn't seem to work.

Unfortunately adding void as a return type causes more type issues than it solves. An example:

// `app/javascript/packages/address-search/components/address-input.tsx`
        <TextInput
          required
          ref={registerField('address')} // This is either undefined or `ForwardedRef<HTMLInputElement>` adding 'void' here seems like a bad idea.

@aduth aduth May 1, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof. Okay, in that case, maybe we could at least inline the default?

  registerField: () => undefined,

@charleyf charleyf May 1, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much better. For some reason I couldn't get the inline function to work before, but now it works. 👍

};

describe('InPersonLocationFullAddressEntryPostOfficeSearchStep', () => {
Expand Down
9 changes: 8 additions & 1 deletion ...ment-capture/components/in-person-location-full-address-entry-post-office-search-step.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,23 @@ import { request } from '@18f/identity-request';
import { forceRedirect } from '@18f/identity-url';
import { FullAddressSearch, transformKeys, snakeCase } from '@18f/identity-address-search';
import type { FormattedLocation } from '@18f/identity-address-search/types';
import type { RegisterFieldCallback } from '@18f/identity-form-steps';
import BackButton from './back-button';
import AnalyticsContext from '../context/analytics';
import { InPersonContext } from '../context';
import UploadContext from '../context/upload';

interface InPersonLocationFullAddressEntryPostOfficeSearchStepProps {
onChange: ({ selectedLocationAddress }: { selectedLocationAddress: string }) => void;
toPreviousStep: () => void;
registerField: RegisterFieldCallback;
}

function InPersonLocationFullAddressEntryPostOfficeSearchStep({
onChange,
toPreviousStep,
registerField,
}) {
}: InPersonLocationFullAddressEntryPostOfficeSearchStepProps) {
const { inPersonURL, locationsURL, usStatesTerritories } = useContext(InPersonContext);
const [inProgress, setInProgress] = useState<boolean>(false);
const [autoSubmit, setAutoSubmit] = useState<boolean>(false);
Expand Down
4 changes: 3 additions & 1 deletion .../packages/document-capture/components/in-person-location-post-office-search-step.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,13 @@ const MULTI_LOCATION_RESPONSE = [
},
];

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const defaultRegisterField = (string) => undefined;
const DEFAULT_PROPS = {
toPreviousStep() {},
onChange() {},
value: {},
registerField() {},
registerField: defaultRegisterField,
};

describe('InPersonLocationPostOfficeSearchStep', () => {
Expand Down
13 changes: 12 additions & 1 deletion ...cript/packages/document-capture/components/in-person-location-post-office-search-step.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,23 @@ import { request } from '@18f/identity-request';
import { forceRedirect } from '@18f/identity-url';
import AddressSearch, { transformKeys, snakeCase } from '@18f/identity-address-search';
import type { FormattedLocation } from '@18f/identity-address-search/types';
import type { RegisterFieldCallback } from '@18f/identity-form-steps';
import BackButton from './back-button';
import AnalyticsContext from '../context/analytics';
import { InPersonContext } from '../context';
import UploadContext from '../context/upload';

function InPersonLocationPostOfficeSearchStep({ onChange, toPreviousStep, registerField }) {
interface InPersonLocationPostOfficeSearchStepProps {

@aduth aduth May 1, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Elsewhere we extend an interface exposed by FormSteps which helps avoid repeating the type signatures, and also surfaces additional props that are available. Maybe we should do the same here and in InPersonPrepareStep and InPersonLocationFullAddressEntryPostOfficeSearchStep?

identity-idp/app/javascript/packages/document-capture/components/review-issues-step.tsx

Line 39 in c386364

interface ReviewIssuesStepProps extends FormStepComponentProps<ReviewIssuesStepValue> {

onChange: ({ selectedLocationAddress }: { selectedLocationAddress: string }) => void;
toPreviousStep: () => void;
registerField: RegisterFieldCallback;
}

function InPersonLocationPostOfficeSearchStep({
onChange,
toPreviousStep,
registerField,
}: InPersonLocationPostOfficeSearchStepProps) {
const { inPersonURL, locationsURL, addressSearchURL } = useContext(InPersonContext);
const [inProgress, setInProgress] = useState<boolean>(false);
const [autoSubmit, setAutoSubmit] = useState<boolean>(false);
Expand Down
2 changes: 1 addition & 1 deletion app/javascript/packages/document-capture/components/in-person-prepare-step.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import InPersonTroubleshootingOptions from './in-person-troubleshooting-options'
import { InPersonContext } from '../context';
import InPersonOutageAlert from './in-person-outage-alert';

function InPersonPrepareStep({ toPreviousStep }) {
function InPersonPrepareStep({ toPreviousStep }: { toPreviousStep: () => void }) {
const { t } = useI18n();
const { flowPath } = useContext(UploadContext);
const { securityAndPrivacyHowItWorksURL } = useContext(MarketingSiteContext);
Expand Down
1 change: 0 additions & 1 deletion scripts/enforce-typescript-files.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ const LEGACY_FILE_EXCEPTIONS = [
'app/javascript/packages/device/index.js',
'app/javascript/packages/document-capture/index.js',
'app/javascript/packages/document-capture/components/acuant-capture-canvas.jsx',
'app/javascript/packages/document-capture/components/acuant-selfie-capture-canvas.jsx',
'app/javascript/packages/document-capture/components/callback-on-mount.jsx',
'app/javascript/packages/document-capture/components/file-image.jsx',
'app/javascript/packages/document-capture/components/submission-interstitial.jsx',
Expand Down