From 086e3997489ea16b1c224f09261c549b3cc05a16 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Mon, 28 Sep 2020 16:24:18 -0400 Subject: [PATCH 1/3] Disable ESLint rule no-use-before-define **Why:** With interdepedent functions, or when using React hooks which are subject to Rules of Hooks, not always possible to define function prior to referenced usage. --- .eslintrc | 1 + 1 file changed, 1 insertion(+) diff --git a/.eslintrc b/.eslintrc index 78cff3067ca..75eb89e716a 100644 --- a/.eslintrc +++ b/.eslintrc @@ -28,6 +28,7 @@ "no-confusing-arrow": "off", "no-plusplus": "off", "no-unused-expressions": "off", + "no-use-before-define": "off", "implicit-arrow-linebreak": "off", "object-curly-newline": "off", "operator-linebreak": "off", From e61e74fb972e11cf1f90cc542da27867d5eb2288 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Mon, 28 Sep 2020 16:25:35 -0400 Subject: [PATCH 2/3] Refactor form step validation as required field **Why**: Reduces responsibility of step where current validation is only required, allows validation of conditionally-registered fields. --- .../components/document-capture.jsx | 12 +- .../components/documents-step.jsx | 18 +-- .../components/form-steps.jsx | 126 +++++++----------- .../components/selfie-step.jsx | 17 +-- .../hooks/use-history-param.js | 8 +- .../components/document-capture-spec.jsx | 6 +- .../components/documents-step-spec.jsx | 35 +---- .../components/form-steps-spec.jsx | 70 ++-------- .../components/selfie-step-spec.jsx | 23 +--- .../hooks/use-history-param-spec.jsx | 9 +- 10 files changed, 80 insertions(+), 244 deletions(-) diff --git a/app/javascript/packages/document-capture/components/document-capture.jsx b/app/javascript/packages/document-capture/components/document-capture.jsx index c306fb6f29c..3286e459ee5 100644 --- a/app/javascript/packages/document-capture/components/document-capture.jsx +++ b/app/javascript/packages/document-capture/components/document-capture.jsx @@ -2,8 +2,8 @@ import React, { useState, useContext } from 'react'; import { Alert } from '@18f/identity-components'; import FormSteps from './form-steps'; import { UploadFormEntriesError } from '../services/upload'; -import DocumentsStep, { validate as validateDocumentsStep } from './documents-step'; -import SelfieStep, { validate as validateSelfieStep } from './selfie-step'; +import DocumentsStep from './documents-step'; +import SelfieStep from './selfie-step'; import MobileIntroStep from './mobile-intro-step'; import DeviceContext from '../context/device'; import Submission from './submission'; @@ -53,13 +53,11 @@ function DocumentCapture({ isLivenessEnabled = true }) { footer: isMobile ? undefined : () =>

{t('doc_auth.info.document_capture_upload_image')}

, - validate: validateDocumentsStep, }, isLivenessEnabled && { name: 'selfie', title: t('doc_auth.headings.selfie'), form: SelfieStep, - validate: validateSelfieStep, }, ].filter(Boolean)); @@ -74,10 +72,6 @@ function DocumentCapture({ isLivenessEnabled = true }) { } const isFormEntriesError = submissionError && submissionError instanceof UploadFormEntriesError; - let initialStep; - if (submissionError) { - initialStep = isFormEntriesError || !isLivenessEnabled ? 'documents' : 'selfie'; - } return formValues && !submissionError ? ( ); diff --git a/app/javascript/packages/document-capture/components/documents-step.jsx b/app/javascript/packages/document-capture/components/documents-step.jsx index 3e2d99b3807..2d339a94707 100644 --- a/app/javascript/packages/document-capture/components/documents-step.jsx +++ b/app/javascript/packages/document-capture/components/documents-step.jsx @@ -1,15 +1,9 @@ import React, { useContext } from 'react'; import AcuantCapture from './acuant-capture'; import FormErrorMessage from './form-error-message'; -import { RequiredValueMissingError } from './form-steps'; import useI18n from '../hooks/use-i18n'; import DeviceContext from '../context/device'; -/** - * @template V - * @typedef {import('./form-steps').FormStepValidateResult} FormStepValidateResult - */ - /** * @typedef DocumentsStepValue * @@ -51,7 +45,7 @@ function DocumentsStep({ return ( } - */ -export function validate(values) { - return DOCUMENT_SIDES.filter((side) => !values[side]).map((side) => ({ - field: side, - error: new RequiredValueMissingError(), - })); -} - export default DocumentsStep; diff --git a/app/javascript/packages/document-capture/components/form-steps.jsx b/app/javascript/packages/document-capture/components/form-steps.jsx index a35f99e65bf..e7109ff03cc 100644 --- a/app/javascript/packages/document-capture/components/form-steps.jsx +++ b/app/javascript/packages/document-capture/components/form-steps.jsx @@ -13,6 +13,12 @@ import useHistoryParam from '../hooks/use-history-param'; * @template V */ +/** + * @typedef FormStepRegisterFieldOptions + * + * @prop {boolean} isRequired Whether field is required. + */ + /** * @typedef FormStepComponentProps * @@ -20,24 +26,15 @@ import useHistoryParam from '../hooks/use-history-param'; * existing values. * @prop {Partial} value Current values. * @prop {FormStepError[]=} errors Current active errors. - * @prop {(field:string)=>undefined|((fieldNode:HTMLElement?)=>void)} registerField Registers field + * @prop {( + * field:string, + * options?:Partial + * )=>undefined|import('react').RefCallback} registerField Registers field * by given name, returning ref assignment function. * * @template V */ -/** - * @template {Record} V - * - * @typedef {FormStepError[]} FormStepValidateResult - */ - -/** - * @template {Record} V - * - * @typedef {(values:V)=>undefined|FormStepValidateResult} FormStepValidate - */ - /** * @typedef FormStep * @@ -45,15 +42,13 @@ import useHistoryParam from '../hooks/use-history-param'; * @prop {string} title Step title, shown as heading. * @prop {import('react').FC>>} form Step form component. * @prop {import('react').FC=} footer Optional step footer component. - * @prop {FormStepValidate>} validate Step validity function. Given set of form - * values, returns an object with keys from form values mapped to an error, if applicable. Returns - * undefined or an empty object if there are no errors. */ /** * @typedef FieldsRefEntry * * @prop {import('react').RefCallback} refCallback Ref callback. + * @prop {boolean} isRequired Whether field is required. * @prop {HTMLElement?=} element Element assigned by ref callback. */ @@ -62,7 +57,7 @@ import useHistoryParam from '../hooks/use-history-param'; * * @prop {FormStep[]=} steps Form steps. * @prop {Record=} initialValues Form values to populate initial state. - * @prop {string=} initialStep Step to start from. + * @prop {boolean=} autoFocus Whether to automatically focus heading on mount. * @prop {(values:Record)=>void=} onComplete Form completion callback. */ @@ -71,30 +66,6 @@ import useHistoryParam from '../hooks/use-history-param'; */ export class RequiredValueMissingError extends Error {} -/** - * Given a step object and current set of form values, returns true if the form values would satisfy - * the validity requirements of the step. - * - * @param {FormStep} step Form step. - * @param {Record} values Current form values. - */ -function getValidationErrors(step, values) { - const { validate = () => undefined } = step; - return validate(values); -} - -/** - * Given a step object and current set of form values, returns true if the form values would satisfy - * the validity requirements of the step. - * - * @param {FormStep} step Form step. - * @param {Record} values Current form values. - */ -export function isStepValid(step, values) { - const errors = getValidationErrors(step, values); - return !errors || !Object.keys(errors).length; -} - /** * Returns the index of the step in the array which matches the given name. Returns `-1` if there is * no step found by that name. @@ -108,32 +79,17 @@ export function getStepIndexByName(steps, name) { return steps.findIndex((step) => step.name === name); } -/** - * Returns the index of the last step in the array where the values satisfy the requirements of the - * step. If all steps are valid, returns the index of the last member. Returns `-1` if all steps are - * invalid, or if the array is empty. - * - * @param {FormStep[]} steps Form steps. - * @param {object} values Current form values. - * - * @return {number} Step index. - */ -export function getLastValidStepIndex(steps, values) { - const index = steps.findIndex((step) => !isStepValid(step, values)); - return index === -1 ? steps.length - 1 : index - 1; -} - /** * @param {FormStepsProps} props Props object. */ -function FormSteps({ steps = [], onComplete = () => {}, initialValues = {}, initialStep }) { +function FormSteps({ steps = [], onComplete = () => {}, initialValues = {}, autoFocus }) { const [values, setValues] = useState(initialValues); const [activeErrors, setActiveErrors] = useState( - /** @type {FormStepValidateResult>=} */ (undefined), + /** @type {FormStepError>[]=} */ (undefined), ); const formRef = useRef(/** @type {?HTMLFormElement} */ (null)); const headingRef = useRef(/** @type {?HTMLHeadingElement} */ (null)); - const [stepName, setStepName] = useHistoryParam('step', initialStep); + const [stepName, setStepName] = useHistoryParam('step', null); const { t } = useI18n(); const fields = useRef(/** @type {Record} */ ({})); const didSubmitWithErrors = useRef(false); @@ -146,23 +102,12 @@ function FormSteps({ steps = [], onComplete = () => {}, initialValues = {}, init didSubmitWithErrors.current = false; }, [activeErrors]); - // An "effective" step is computed in consideration of the facts that (1) there may be no history - // parameter present, in which case the first step should be used, and (2) the values may not be - // valid for previous steps, in which case the furthest valid step should be set. - const effectiveStepIndex = Math.max( - Math.min(getStepIndexByName(steps, stepName), getLastValidStepIndex(steps, values) + 1), - 0, - ); - const effectiveStep = steps[effectiveStepIndex]; - useEffect(() => { - // The effective step is used in the initial render, but since it may be out of sync with the - // history parameter, it is synced after mount. - if (effectiveStep && stepName && effectiveStep.name !== stepName) { - setStepName(effectiveStep.name); - } + const stepIndex = Math.max(getStepIndexByName(steps, stepName), 0); + const step = steps[stepIndex]; + useEffect(() => { // Treat explicit initial step the same as step transition, placing focus to header. - if (initialStep && headingRef.current) { + if (autoFocus && headingRef.current) { headingRef.current.focus(); } }, []); @@ -171,16 +116,34 @@ function FormSteps({ steps = [], onComplete = () => {}, initialValues = {}, init // Errors are assigned at the first attempt to submit. Once errors are assigned, track value // changes to remove validation errors as they become resolved. if (activeErrors) { - const nextActiveErrors = getValidationErrors(effectiveStep, values); + const nextActiveErrors = getValidationErrors(); setActiveErrors(nextActiveErrors); } }, [values]); // An empty steps array is allowed, in which case there is nothing to render. - if (!effectiveStep) { + if (!step) { return null; } + /** + * Returns array of form errors for the current set of values. + * + * @return {FormStepError>[]} + */ + function getValidationErrors() { + return Object.keys(fields.current).reduce((result, key) => { + const { element, isRequired } = fields.current[key]; + const isActive = !!element; + + if (isActive && isRequired && !values[key]) { + result = result.concat({ field: key, error: new RequiredValueMissingError() }); + } + + return result; + }, /** @type {FormStepError>[]} */ ([])); + } + /** * Increments state to the next step, or calls onComplete callback if the current step is the last * step. @@ -190,14 +153,14 @@ function FormSteps({ steps = [], onComplete = () => {}, initialValues = {}, init function toNextStep(event) { event.preventDefault(); - const nextActiveErrors = getValidationErrors(effectiveStep, values); + const nextActiveErrors = getValidationErrors(); setActiveErrors(nextActiveErrors); didSubmitWithErrors.current = true; if (nextActiveErrors?.length) { return; } - const nextStepIndex = effectiveStepIndex + 1; + const nextStepIndex = stepIndex + 1; const isComplete = nextStepIndex === steps.length; if (isComplete) { // Clear step parameter from URL. @@ -211,8 +174,8 @@ function FormSteps({ steps = [], onComplete = () => {}, initialValues = {}, init headingRef.current?.focus(); } - const { form: Form, footer: Footer, name, title } = effectiveStep; - const isLastStep = effectiveStepIndex + 1 === steps.length; + const { form: Form, footer: Footer, name, title } = step; + const isLastStep = stepIndex + 1 === steps.length; return (
@@ -226,12 +189,13 @@ function FormSteps({ steps = [], onComplete = () => {}, initialValues = {}, init onChange={(nextValuesPatch) => { setValues((prevValues) => ({ ...prevValues, ...nextValuesPatch })); }} - registerField={(field) => { + registerField={(field, options = {}) => { if (!fields.current[field]) { fields.current[field] = { refCallback(fieldNode) { fields.current[field].element = fieldNode; }, + isRequired: !!options.isRequired, }; } diff --git a/app/javascript/packages/document-capture/components/selfie-step.jsx b/app/javascript/packages/document-capture/components/selfie-step.jsx index 635f0e83d49..ad3f3f7535c 100644 --- a/app/javascript/packages/document-capture/components/selfie-step.jsx +++ b/app/javascript/packages/document-capture/components/selfie-step.jsx @@ -1,17 +1,11 @@ import React, { useContext } from 'react'; import { hasMediaAccess } from '@18f/identity-device'; -import { RequiredValueMissingError } from './form-steps'; import useI18n from '../hooks/use-i18n'; import DeviceContext from '../context/device'; import AcuantCapture from './acuant-capture'; import SelfieCapture from './selfie-capture'; import FormErrorMessage from './form-error-message'; -/** - * @template V - * @typedef {import('./form-steps').FormStepValidateResult} FormStepValidateResult - */ - /** * @typedef SelfieStepValue * @@ -42,7 +36,7 @@ function SelfieStep({ {isMobile || !hasMediaAccess() ? ( ) : ( onChange({ selfie: nextSelfie })} errorMessage={error ? : undefined} @@ -64,11 +58,4 @@ function SelfieStep({ ); } -/** - * @type {import('./form-steps').FormStepValidate} - */ -export function validate(values) { - return values.selfie ? [] : [{ field: 'selfie', error: new RequiredValueMissingError() }]; -} - export default SelfieStep; diff --git a/app/javascript/packages/document-capture/hooks/use-history-param.js b/app/javascript/packages/document-capture/hooks/use-history-param.js index 81d27099c91..a4b1a3cf568 100644 --- a/app/javascript/packages/document-capture/hooks/use-history-param.js +++ b/app/javascript/packages/document-capture/hooks/use-history-param.js @@ -55,8 +55,8 @@ function scrollTo(left, top) { * * @see https://developer.mozilla.org/en-US/docs/Web/API/History/pushState * - * @param {string} name Parameter name to sync. - * @param {string=} initialValue Value to use as initial in absence of another value. + * @param {string} name Parameter name to sync. + * @param {string?=} initialValue Value to use as initial in absence of another value. * * @return {[any,(nextParamValue:any)=>void]} Tuple of current state, state setter. */ @@ -87,8 +87,8 @@ function useHistoryParam(name, initialValue) { setValue(getCurrentQueryParam()); } - if (value === undefined && initialValue) { - setValue(initialValue); + if (initialValue !== undefined) { + setValue(initialValue ?? undefined); window.history.replaceState(null, '', getValueURL(initialValue)); } diff --git a/spec/javascripts/packages/document-capture/components/document-capture-spec.jsx b/spec/javascripts/packages/document-capture/components/document-capture-spec.jsx index 97dc43bf98e..dcdca0380e2 100644 --- a/spec/javascripts/packages/document-capture/components/document-capture-spec.jsx +++ b/spec/javascripts/packages/document-capture/components/document-capture-spec.jsx @@ -188,7 +188,7 @@ describe('document-capture/components/document-capture', () => { initialize({ isCameraSupported: false }); window.AcuantPassiveLiveness.startSelfieCapture.callsArgWithAsync(0, ''); - const continueButton = getByText('forms.buttons.continue'); + let continueButton = getByText('forms.buttons.continue'); userEvent.click(continueButton); await findAllByText('simple_form.required.text'); userEvent.upload( @@ -218,8 +218,10 @@ describe('document-capture/components/document-capture', () => { /React will try to recreate this component tree from scratch using the error boundary you provided/, ); - const heading = getByText('doc_auth.headings.selfie'); + const heading = getByText('doc_auth.headings.document_capture'); expect(document.activeElement).to.equal(heading); + continueButton = getByText('forms.buttons.continue'); + userEvent.click(continueButton); const hasValueSelected = !!getByText('doc_auth.forms.change_file'); expect(hasValueSelected).to.be.true(); diff --git a/spec/javascripts/packages/document-capture/components/documents-step-spec.jsx b/spec/javascripts/packages/document-capture/components/documents-step-spec.jsx index 7bfeb66fdca..c6f2c24e0ed 100644 --- a/spec/javascripts/packages/document-capture/components/documents-step-spec.jsx +++ b/spec/javascripts/packages/document-capture/components/documents-step-spec.jsx @@ -2,43 +2,10 @@ import React from 'react'; import userEvent from '@testing-library/user-event'; import sinon from 'sinon'; import DeviceContext from '@18f/identity-document-capture/context/device'; -import DocumentsStep, { validate } from '@18f/identity-document-capture/components/documents-step'; -import { RequiredValueMissingError } from '@18f/identity-document-capture/components/form-steps'; +import DocumentsStep from '@18f/identity-document-capture/components/documents-step'; import render from '../../../support/render'; describe('document-capture/components/documents-step', () => { - describe('validate', () => { - it('returns errors if both front and back are unset', () => { - const value = {}; - const result = validate(value); - - expect(result).to.have.lengthOf(2); - expect(result[0].field).to.equal('front'); - expect(result[0].error).to.be.instanceOf(RequiredValueMissingError); - expect(result[1].field).to.equal('back'); - expect(result[1].error).to.be.instanceOf(RequiredValueMissingError); - }); - - it('returns error if one of front and back are unset', () => { - const value = { front: new window.File([], 'upload.png', { type: 'image/png' }) }; - const result = validate(value); - - expect(result).to.have.lengthOf(1); - expect(result[0].field).to.equal('back'); - expect(result[0].error).to.be.instanceOf(RequiredValueMissingError); - }); - - it('returns empty array if both front and back are set', () => { - const value = { - front: new window.File([], 'upload.png', { type: 'image/png' }), - back: new window.File([], 'upload.png', { type: 'image/png' }), - }; - const result = validate(value); - - expect(result).to.deep.equal([]); - }); - }); - it('renders with front and back inputs', () => { const { getByLabelText } = render(); diff --git a/spec/javascripts/packages/document-capture/components/form-steps-spec.jsx b/spec/javascripts/packages/document-capture/components/form-steps-spec.jsx index e9b64fcdb15..ca15d045cbe 100644 --- a/spec/javascripts/packages/document-capture/components/form-steps-spec.jsx +++ b/spec/javascripts/packages/document-capture/components/form-steps-spec.jsx @@ -2,9 +2,7 @@ import React from 'react'; import userEvent from '@testing-library/user-event'; import sinon from 'sinon'; import FormSteps, { - isStepValid, getStepIndexByName, - getLastValidStepIndex, } from '@18f/identity-document-capture/components/form-steps'; import render from '../../../support/render'; @@ -19,7 +17,7 @@ describe('document-capture/components/form-steps', () => { ), - validate: (value) => (value.second ? [] : [{ field: 'second', error: new Error() }]), }, { name: 'last', title: 'Last Title', form: () => Last }, ]; @@ -43,27 +40,6 @@ describe('document-capture/components/form-steps', () => { window.location.hash = originalHash; }); - describe('isStepValid', () => { - it('defaults to true if there is no specified validity function', () => { - const step = { name: 'example' }; - - const result = isStepValid(step, {}); - - expect(result).to.be.true(); - }); - - it('returns the result of the validity function given form values', () => { - const step = { - name: 'example', - validate: (values) => (values.ok ? undefined : { ok: new Error() }), - }; - - const result = isStepValid(step, { ok: false }); - - expect(result).to.be.false(); - }); - }); - describe('getStepIndexByName', () => { it('returns -1 if no step by name', () => { const result = getStepIndexByName(STEPS, 'third'); @@ -78,30 +54,6 @@ describe('document-capture/components/form-steps', () => { }); }); - describe('getLastValidStepIndex', () => { - it('returns -1 if array is empty', () => { - const result = getLastValidStepIndex([], {}); - - expect(result).to.be.equal(-1); - }); - - it('returns -1 if all steps are invalid', () => { - const steps = [...STEPS].map((step) => ({ - ...step, - validate: () => ({ missing: new Error() }), - })); - const result = getLastValidStepIndex(steps, {}); - - expect(result).to.be.equal(-1); - }); - - it('returns index of the last valid step', () => { - const result = getLastValidStepIndex(STEPS, { second: 'valid' }); - - expect(result).to.be.equal(2); - }); - }); - it('renders nothing if given empty steps array', () => { const { container } = render(); @@ -217,35 +169,35 @@ describe('document-capture/components/form-steps', () => { expect(document.activeElement).to.equal(originalActiveElement); }); - it('validates step completion', () => { + it('resets to first step at mount', () => { window.location.hash = '#step=last'; render(); - expect(window.location.hash).to.equal('#step=second'); + expect(window.location.hash).to.equal(''); }); - it('accepts initial step', () => { - const { getByText } = render(); + it('optionally auto-focuses', () => { + const { getByText } = render(); - expect(window.location.hash).to.equal('#step=second'); - expect(document.activeElement).to.equal(getByText('Second Title')); - expect(getByText('Second')).to.be.ok(); + expect(document.activeElement).to.equal(getByText('First Title')); }); it('accepts initial values', () => { - const { getByLabelText } = render( - , + const { getByText, getByLabelText } = render( + , ); + userEvent.click(getByText('forms.buttons.continue')); const input = getByLabelText('Second'); expect(input.value).to.equal('prefilled'); }); it('prevents submission if step is invalid', () => { - const { getByText, getByLabelText } = render(); + const { getByText, getByLabelText } = render(); + userEvent.click(getByText('forms.buttons.continue')); userEvent.click(getByText('forms.buttons.continue')); expect(window.location.hash).to.equal('#step=second'); diff --git a/spec/javascripts/packages/document-capture/components/selfie-step-spec.jsx b/spec/javascripts/packages/document-capture/components/selfie-step-spec.jsx index 1c94cbcac9b..0dd2decb80a 100644 --- a/spec/javascripts/packages/document-capture/components/selfie-step-spec.jsx +++ b/spec/javascripts/packages/document-capture/components/selfie-step-spec.jsx @@ -3,34 +3,13 @@ import userEvent from '@testing-library/user-event'; import { waitFor } from '@testing-library/dom'; import sinon from 'sinon'; import { AcuantProvider } from '@18f/identity-document-capture'; -import SelfieStep, { validate } from '@18f/identity-document-capture/components/selfie-step'; -import { RequiredValueMissingError } from '@18f/identity-document-capture/components/form-steps'; +import SelfieStep from '@18f/identity-document-capture/components/selfie-step'; import render from '../../../support/render'; import { useAcuant } from '../../../support/acuant'; describe('document-capture/components/selfie-step', () => { const { initialize } = useAcuant(); - describe('validate', () => { - it('returns object with error if selfie is unset', () => { - const value = {}; - const result = validate(value); - - expect(result).to.have.lengthOf(1); - expect(result[0].field).to.equal('selfie'); - expect(result[0].error).to.be.instanceOf(RequiredValueMissingError); - }); - - it('returns empty array if selfie is set', () => { - const value = { - selfie: new window.File([], 'upload.png', { type: 'image/png' }), - }; - const result = validate(value); - - expect(result).to.deep.equal([]); - }); - }); - it('calls onChange callback with uploaded image', async () => { const onChange = sinon.stub(); const { getByLabelText } = render( diff --git a/spec/javascripts/packages/document-capture/hooks/use-history-param-spec.jsx b/spec/javascripts/packages/document-capture/hooks/use-history-param-spec.jsx index 33b0301aaf0..8f2051f54aa 100644 --- a/spec/javascripts/packages/document-capture/hooks/use-history-param-spec.jsx +++ b/spec/javascripts/packages/document-capture/hooks/use-history-param-spec.jsx @@ -68,13 +68,20 @@ describe('useHistoryParam', () => { expect(getByDisplayValue('5')).to.be.ok(); }); - it('accepts an initial value to use in absence of an initial URL', () => { + it('accepts an initial value', () => { const { getByDisplayValue } = render(); expect(window.location.hash).to.equal('#the%20count=5'); expect(getByDisplayValue('5')).to.be.ok(); }); + it('accepts empty initial value', () => { + const { getByDisplayValue } = render(); + + expect(window.location.hash).to.equal(''); + expect(getByDisplayValue('0')).to.be.ok(); + }); + it('syncs by setter', () => { const { getByText, getByDisplayValue } = render(); From 3f035d5fc754f58dc7950d2e0a187ac170505218 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 29 Sep 2020 10:34:46 -0400 Subject: [PATCH 3/3] Define getValidationErrors before useEffect hook See: https://github.com/18F/identity-idp/pull/4243#discussion_r496762059 --- .eslintrc | 1 - .../components/form-steps.jsx | 28 +++++++++---------- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/.eslintrc b/.eslintrc index 75eb89e716a..78cff3067ca 100644 --- a/.eslintrc +++ b/.eslintrc @@ -28,7 +28,6 @@ "no-confusing-arrow": "off", "no-plusplus": "off", "no-unused-expressions": "off", - "no-use-before-define": "off", "implicit-arrow-linebreak": "off", "object-curly-newline": "off", "operator-linebreak": "off", diff --git a/app/javascript/packages/document-capture/components/form-steps.jsx b/app/javascript/packages/document-capture/components/form-steps.jsx index e7109ff03cc..beacbe146aa 100644 --- a/app/javascript/packages/document-capture/components/form-steps.jsx +++ b/app/javascript/packages/document-capture/components/form-steps.jsx @@ -112,20 +112,6 @@ function FormSteps({ steps = [], onComplete = () => {}, initialValues = {}, auto } }, []); - useEffect(() => { - // Errors are assigned at the first attempt to submit. Once errors are assigned, track value - // changes to remove validation errors as they become resolved. - if (activeErrors) { - const nextActiveErrors = getValidationErrors(); - setActiveErrors(nextActiveErrors); - } - }, [values]); - - // An empty steps array is allowed, in which case there is nothing to render. - if (!step) { - return null; - } - /** * Returns array of form errors for the current set of values. * @@ -144,6 +130,20 @@ function FormSteps({ steps = [], onComplete = () => {}, initialValues = {}, auto }, /** @type {FormStepError>[]} */ ([])); } + useEffect(() => { + // Errors are assigned at the first attempt to submit. Once errors are assigned, track value + // changes to remove validation errors as they become resolved. + if (activeErrors) { + const nextActiveErrors = getValidationErrors(); + setActiveErrors(nextActiveErrors); + } + }, [values]); + + // An empty steps array is allowed, in which case there is nothing to render. + if (!step) { + return null; + } + /** * Increments state to the next step, or calls onComplete callback if the current step is the last * step.