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: <img width="817" alt="Screenshot 2024-10-15 at 21 30 06" src="https://github.com/user-attachments/assets/fd5d1329-5668-4247-8094-30f6b7b77d85">
dnbexperience · Oct 16, 2024 · dada40a · dada40a
1 parent f70ba3d
commit dada40a
Show file tree

Hide file tree

Showing 11 changed files with 120 additions and 44 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -499,7 +499,6 @@ describe('Iterate.Array', () => {
               { mem: 'D', second: '2nd' },
             ]}
           >
-            <Field.String itemPath="/" />
             {renderProp1}
             {renderProp2}
           </Iterate.Array>

diff --git a/...ages/dnb-eufemia/src/extensions/forms/Wizard/Container/__tests__/WizardContainer.test.tsx b/...ages/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
@@ -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(
+        <React.StrictMode>
+          <MockComponent />
+          <MockComponent />
+        </React.StrictMode>
+      )
+
+      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(
+        <React.StrictMode>
+          <Field.String path="/myPath" />
+          <Field.String path="/myPath" />
+        </React.StrictMode>
+      )
+
+      expect(log).toHaveBeenCalledTimes(0)
+      process.env.NODE_ENV = originalNodeEnv
+    })
+
     it('for the "itemPath" prop', () => {
       render(
         <React.StrictMode>
-          <Iterate.Array value={['foo']}>
-            <Field.String itemPath="/myPath" />
-            <Field.String itemPath="/myPath" />
+          <Iterate.Array value={['foo', 'bar']}>
+            <Field.String itemPath="/myPath" defaultValue="foo" />
+            <Field.String itemPath="/myPath" defaultValue="bar" />
           </Iterate.Array>
         </React.StrictMode>
       )
 
       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(
         <React.StrictMode>
           <Iterate.Array value={['foo']}>
-            <Field.String itemPath="/myPath" />
+            <Field.String itemPath="/myPath" defaultValue="foo" />
           </Iterate.Array>
           <Iterate.Array value={['bar']}>
-            <Field.String itemPath="/myPath" />
+            <Field.String itemPath="/myPath" defaultValue="bar" />
           </Iterate.Array>
         </React.StrictMode>
       )
 
       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(
+        <React.StrictMode>
+          <Iterate.Array value={['foo']}>
+            <Field.String path="/myPath" />
+          </Iterate.Array>
+        </React.StrictMode>
+      )
+
+      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
@@ -88,6 +88,7 @@ export default function useFieldProps<Value, EmptyValue, Props>(
   {
     executeOnChangeRegardlessOfError = false,
     updateContextDataInSync = false,
+    omitMultiplePathWarning = false,
   } = {}
 ): typeof localeProps & ReturnAdditional<Value> {
   const { extend } = useContext(FieldProviderContext)
@@ -1547,17 +1548,28 @@ export default function useFieldProps<Value, EmptyValue, Props>(
 
   // - 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 () => {
           existingFields.delete(identifier)
         }
       }
     }
-  }, [hasItemPath, hasPath, identifier])
+  }, [
+    hasItemPath,
+    hasPath,
+    identifier,
+    iterateItemContext,
+    omitMultiplePathWarning,
+  ])
 
   useEffect(() => {
     return () => {