From dada40ac2bdd0720fc618734ac5357449d9a79f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20H=C3=B8egh?= Date: Wed, 16 Oct 2024 09:36:08 +0200 Subject: [PATCH] chore: enhance multiple path warning (#4130) - Shorten the message. - Avoid showing the warning in our logs. - Avoid showing the warning when not needed. - Use `spyOnEufemiaWarn` in other tests as well. The origin of this PR: Screenshot 2024-10-15 at 21 30 06 --- .../dnb-eufemia/src/core/jest/jestSetup.js | 13 +++ .../Provider/__tests__/Provider.test.tsx | 17 ++-- .../extensions/forms/Field/Slider/Slider.tsx | 4 +- .../Form/Handler/__tests__/Handler.test.tsx | 9 +-- .../Isolation/__tests__/Isolation.test.tsx | 9 +++ .../Form/Section/__tests__/Section.test.tsx | 5 ++ .../extensions/forms/Iterate/Array/Array.tsx | 1 + .../Iterate/Array/__tests__/Array.test.tsx | 1 - .../__tests__/WizardContainer.test.tsx | 6 +- .../hooks/__tests__/useFieldProps.test.tsx | 81 +++++++++++++------ .../extensions/forms/hooks/useFieldProps.ts | 18 ++++- 11 files changed, 120 insertions(+), 44 deletions(-) diff --git a/packages/dnb-eufemia/src/core/jest/jestSetup.js b/packages/dnb-eufemia/src/core/jest/jestSetup.js index e049a4951d8..28ea734360c 100644 --- a/packages/dnb-eufemia/src/core/jest/jestSetup.js +++ b/packages/dnb-eufemia/src/core/jest/jestSetup.js @@ -129,3 +129,16 @@ export const axeComponent = async (...components) => { typeof components[1] === 'object' ? components[1] : null ) } + +export function spyOnEufemiaWarn() { + const originalConsoleLog = console.log + const log = jest + .spyOn(console, 'log') + .mockImplementation((...message) => { + if (!message[0].includes('Eufemia')) { + originalConsoleLog(...message) + } + }) + + return log +} diff --git a/packages/dnb-eufemia/src/extensions/forms/DataContext/Provider/__tests__/Provider.test.tsx b/packages/dnb-eufemia/src/extensions/forms/DataContext/Provider/__tests__/Provider.test.tsx index 59614058f78..76136b7ea69 100644 --- a/packages/dnb-eufemia/src/extensions/forms/DataContext/Provider/__tests__/Provider.test.tsx +++ b/packages/dnb-eufemia/src/extensions/forms/DataContext/Provider/__tests__/Provider.test.tsx @@ -8,7 +8,7 @@ import { waitFor, } from '@testing-library/react' import userEvent from '@testing-library/user-event' -import { wait } from '../../../../../core/jest/jestSetup' +import { spyOnEufemiaWarn, wait } from '../../../../../core/jest/jestSetup' import { simulateAnimationEnd } from '../../../../../components/height-animation/__tests__/HeightAnimationUtils' import { GlobalStatus } from '../../../../../components' import { makeUniqueId } from '../../../../../shared/component-helper' @@ -942,12 +942,7 @@ describe('DataContext.Provider', () => { describe('async submit', () => { let log: jest.SpyInstance beforeEach(() => { - const originalConsoleLog = console.log - log = jest.spyOn(console, 'log').mockImplementation((...message) => { - if (!message[0].includes('Eufemia')) { - originalConsoleLog(...message) - } - }) + log = spyOnEufemiaWarn() }) afterEach(() => { log.mockRestore() @@ -3384,6 +3379,8 @@ describe('DataContext.Provider', () => { }) it('should contain data on first render, when nested and in side car', () => { + const log = spyOnEufemiaWarn() + const initialData = { foo: 'bar' } const sidecarMockData = [] const nestedMockData = [] @@ -3424,6 +3421,8 @@ describe('DataContext.Provider', () => { ) expect(sidecar).toHaveValue('') // Because the field is outside of the context expect(nested).toHaveValue('bar') + + log.mockRestore() }) it('should be able to update data from side car', async () => { @@ -3481,6 +3480,8 @@ describe('DataContext.Provider', () => { }) it('should support StrictMode', async () => { + const log = spyOnEufemiaWarn() + const initialData = { foo: 'bar' } const sidecarMockData = [] const nestedMockData = [] @@ -3528,6 +3529,8 @@ describe('DataContext.Provider', () => { ) expect(sidecar).toHaveValue('') // Because the field is outside of the context expect(nested).toHaveValue('bar') + + log.mockRestore() }) it('should set Provider data when sessionStorageId was given', () => { diff --git a/packages/dnb-eufemia/src/extensions/forms/Field/Slider/Slider.tsx b/packages/dnb-eufemia/src/extensions/forms/Field/Slider/Slider.tsx index b2612e42ad6..001e07c6ab1 100644 --- a/packages/dnb-eufemia/src/extensions/forms/Field/Slider/Slider.tsx +++ b/packages/dnb-eufemia/src/extensions/forms/Field/Slider/Slider.tsx @@ -85,7 +85,9 @@ function SliderComponent(props: Props) { handleChange, handleFocus, handleBlur, - } = useFieldProps(preparedProps) + } = useFieldProps(preparedProps, { + omitMultiplePathWarning: true, + }) const handleLocalChange = useCallback( ({ value }: { value: number | number[] }) => { diff --git a/packages/dnb-eufemia/src/extensions/forms/Form/Handler/__tests__/Handler.test.tsx b/packages/dnb-eufemia/src/extensions/forms/Form/Handler/__tests__/Handler.test.tsx index eac88cfdc11..021ec9bd206 100644 --- a/packages/dnb-eufemia/src/extensions/forms/Form/Handler/__tests__/Handler.test.tsx +++ b/packages/dnb-eufemia/src/extensions/forms/Form/Handler/__tests__/Handler.test.tsx @@ -3,7 +3,7 @@ import React from 'react' import { fireEvent, render, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' -import { wait } from '../../../../../core/jest/jestSetup' +import { spyOnEufemiaWarn, wait } from '../../../../../core/jest/jestSetup' import { Form, Field, @@ -491,12 +491,7 @@ describe('Form.Handler', () => { describe('async submit', () => { let log: jest.SpyInstance beforeEach(() => { - const originalConsoleLog = console.log - log = jest.spyOn(console, 'log').mockImplementation((...message) => { - if (!message[0].includes('Eufemia')) { - originalConsoleLog(...message) - } - }) + log = spyOnEufemiaWarn() }) afterEach(() => { log.mockRestore() diff --git a/packages/dnb-eufemia/src/extensions/forms/Form/Isolation/__tests__/Isolation.test.tsx b/packages/dnb-eufemia/src/extensions/forms/Form/Isolation/__tests__/Isolation.test.tsx index 7694dd89ad6..c9a9b8ca421 100644 --- a/packages/dnb-eufemia/src/extensions/forms/Form/Isolation/__tests__/Isolation.test.tsx +++ b/packages/dnb-eufemia/src/extensions/forms/Form/Isolation/__tests__/Isolation.test.tsx @@ -1,4 +1,5 @@ import React from 'react' +import { spyOnEufemiaWarn } from '../../../../../core/jest/jestSetup' import { act, createEvent, @@ -14,6 +15,14 @@ import nbNO from '../../../constants/locales/nb-NO' const nb = nbNO['nb-NO'] describe('Form.Isolation', () => { + let log = null + beforeEach(() => { + log = spyOnEufemiaWarn() + }) + afterEach(() => { + log.mockRestore() + }) + it('should have constant of _supportsSpacingProps="undefined"', () => { expect(Form.Isolation._supportsSpacingProps).toBeUndefined() }) diff --git a/packages/dnb-eufemia/src/extensions/forms/Form/Section/__tests__/Section.test.tsx b/packages/dnb-eufemia/src/extensions/forms/Form/Section/__tests__/Section.test.tsx index 4c2992accd1..e33495ce929 100644 --- a/packages/dnb-eufemia/src/extensions/forms/Form/Section/__tests__/Section.test.tsx +++ b/packages/dnb-eufemia/src/extensions/forms/Form/Section/__tests__/Section.test.tsx @@ -1,5 +1,6 @@ /* eslint-disable @typescript-eslint/ban-ts-comment */ import React from 'react' +import { spyOnEufemiaWarn } from '../../../../../core/jest/jestSetup' import { fireEvent, render } from '@testing-library/react' import { Field, Form, JSONSchema, Tools, Value } from '../../..' import { SectionProps } from '../Section' @@ -1155,6 +1156,8 @@ describe('Form.Section', () => { }) it('should not mutate translations object', () => { + const log = spyOnEufemiaWarn() + const sectionTranslations = { 'nb-NO': { MySection: { CustomField: { label: 'Section nb' } } }, 'en-GB': { MySection: { CustomField: { label: 'Section en' } } }, @@ -1207,6 +1210,8 @@ describe('Form.Section', () => { expect( sectionTranslations['en-GB'].MySection.CustomField.label ).toBe('Section en') + + log.mockRestore() }) }) diff --git a/packages/dnb-eufemia/src/extensions/forms/Iterate/Array/Array.tsx b/packages/dnb-eufemia/src/extensions/forms/Iterate/Array/Array.tsx index 93706831626..816e6e09b8f 100644 --- a/packages/dnb-eufemia/src/extensions/forms/Iterate/Array/Array.tsx +++ b/packages/dnb-eufemia/src/extensions/forms/Iterate/Array/Array.tsx @@ -103,6 +103,7 @@ function ArrayComponent(props: Props) { // To ensure the defaultValue set on the Iterate.Array is set in the data context, // and will not overwrite defaultValues set by fields inside the Iterate.Array. updateContextDataInSync: true, + omitMultiplePathWarning: true, }) useMountEffect(() => { diff --git a/packages/dnb-eufemia/src/extensions/forms/Iterate/Array/__tests__/Array.test.tsx b/packages/dnb-eufemia/src/extensions/forms/Iterate/Array/__tests__/Array.test.tsx index 433d86a1fe1..2e72ced06da 100644 --- a/packages/dnb-eufemia/src/extensions/forms/Iterate/Array/__tests__/Array.test.tsx +++ b/packages/dnb-eufemia/src/extensions/forms/Iterate/Array/__tests__/Array.test.tsx @@ -499,7 +499,6 @@ describe('Iterate.Array', () => { { mem: 'D', second: '2nd' }, ]} > - {renderProp1} {renderProp2} diff --git a/packages/dnb-eufemia/src/extensions/forms/Wizard/Container/__tests__/WizardContainer.test.tsx b/packages/dnb-eufemia/src/extensions/forms/Wizard/Container/__tests__/WizardContainer.test.tsx index 74a06cb1e1e..26eb545e193 100644 --- a/packages/dnb-eufemia/src/extensions/forms/Wizard/Container/__tests__/WizardContainer.test.tsx +++ b/packages/dnb-eufemia/src/extensions/forms/Wizard/Container/__tests__/WizardContainer.test.tsx @@ -1,7 +1,7 @@ import React from 'react' import { fireEvent, render, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' -import { wait } from '../../../../../core/jest/jestSetup' +import { spyOnEufemiaWarn, wait } from '../../../../../core/jest/jestSetup' import { Field, Form, Iterate, OnSubmit, Wizard } from '../../..' import nbNO from '../../../constants/locales/nb-NO' @@ -2296,6 +2296,8 @@ describe('Wizard.Container', () => { }) it('should remember an entered value between step changes', async () => { + const log = spyOnEufemiaWarn() + const onChange = jest.fn() const onStepChange = jest.fn() @@ -2345,6 +2347,8 @@ describe('Wizard.Container', () => { expect.anything() ) expect(document.querySelector('input')).toHaveValue('1234') + + log.mockRestore() }) it('should set defaultValue of Iterate.Array only once between step changes', async () => { diff --git a/packages/dnb-eufemia/src/extensions/forms/hooks/__tests__/useFieldProps.test.tsx b/packages/dnb-eufemia/src/extensions/forms/hooks/__tests__/useFieldProps.test.tsx index e1db6be2982..9b0df59efda 100644 --- a/packages/dnb-eufemia/src/extensions/forms/hooks/__tests__/useFieldProps.test.tsx +++ b/packages/dnb-eufemia/src/extensions/forms/hooks/__tests__/useFieldProps.test.tsx @@ -21,7 +21,7 @@ import Field, { SubmitState, UseFieldProps, } from '../../Forms' -import { wait } from '../../../../core/jest/jestSetup' +import { spyOnEufemiaWarn, wait } from '../../../../core/jest/jestSetup' import { useSharedState } from '../../../../shared/helpers/useSharedState' describe('useFieldProps', () => { @@ -4605,14 +4605,7 @@ describe('useFieldProps', () => { }) it('should set isMounted to true when Wizard step has changed', () => { - const originalConsoleLog = console.log - const log = jest - .spyOn(console, 'log') - .mockImplementation((...message) => { - if (!message[0].includes('Eufemia')) { - originalConsoleLog(...message) - } - }) + const log = spyOnEufemiaWarn() const setMountedFieldState = jest.fn() let activeIndex = 0 @@ -4678,14 +4671,8 @@ describe('useFieldProps', () => { describe('warn about duplicated paths', () => { let log = null - const originalConsoleLog = console.log - beforeEach(() => { - log = jest.spyOn(console, 'log').mockImplementation((...message) => { - if (!message[0].includes('Eufemia')) { - originalConsoleLog(...message) - } - }) + log = spyOnEufemiaWarn() }) afterEach(() => { log.mockRestore() @@ -4701,24 +4688,58 @@ describe('useFieldProps', () => { expect(log).toHaveBeenCalledWith( expect.any(String), - 'You have declared the same path several times:', + 'Path declared multiple times: times:', '/myPath' ) }) + it('should not warn when omitMultiplePathWarning is true', () => { + const MockComponent = () => { + useFieldProps( + { path: '/myPath' }, + { omitMultiplePathWarning: true } + ) + return null + } + + render( + + + + + ) + + expect(log).toHaveBeenCalledTimes(0) + }) + + it('should not warn when process.env.NODE_ENV is not production', () => { + const originalNodeEnv = process.env.NODE_ENV + process.env.NODE_ENV = 'production' + + render( + + + + + ) + + expect(log).toHaveBeenCalledTimes(0) + process.env.NODE_ENV = originalNodeEnv + }) + it('for the "itemPath" prop', () => { render( - - - + + + ) expect(log).toHaveBeenCalledWith( expect.any(String), - 'You have declared the same path several times:', + 'Path declared multiple times: times:', '/0/myPath' ) }) @@ -4727,19 +4748,31 @@ describe('useFieldProps', () => { render( - + - + ) expect(log).toHaveBeenCalledWith( expect.any(String), - 'You have declared the same path several times:', + 'Path declared multiple times: times:', '/0/myPath' ) }) + + it('should not warn when path is used in iterate', () => { + render( + + + + + + ) + + expect(log).toHaveBeenCalledTimes(0) + }) }) }) diff --git a/packages/dnb-eufemia/src/extensions/forms/hooks/useFieldProps.ts b/packages/dnb-eufemia/src/extensions/forms/hooks/useFieldProps.ts index 6288cc94c2c..0de31405f52 100644 --- a/packages/dnb-eufemia/src/extensions/forms/hooks/useFieldProps.ts +++ b/packages/dnb-eufemia/src/extensions/forms/hooks/useFieldProps.ts @@ -88,6 +88,7 @@ export default function useFieldProps( { executeOnChangeRegardlessOfError = false, updateContextDataInSync = false, + omitMultiplePathWarning = false, } = {} ): typeof localeProps & ReturnAdditional { const { extend } = useContext(FieldProviderContext) @@ -1547,9 +1548,14 @@ export default function useFieldProps( // - Warn when a field path is used multiple times useEffect(() => { - if (hasPath || hasItemPath) { + if ( + !omitMultiplePathWarning && + process.env.NODE_ENV !== 'production' && + (hasPath || hasItemPath) && + (hasPath ? !iterateItemContext : true) + ) { if (existingFields.has(identifier)) { - warn('You have declared the same path several times:', identifier) + warn('Path declared multiple times: times:', identifier) } else { existingFields.set(identifier, true) return () => { @@ -1557,7 +1563,13 @@ export default function useFieldProps( } } } - }, [hasItemPath, hasPath, identifier]) + }, [ + hasItemPath, + hasPath, + identifier, + iterateItemContext, + omitMultiplePathWarning, + ]) useEffect(() => { return () => {