fix(core): sanitize React key warning getter in dev

[ogolknev]

fix #641

[veged]

Проблему подтвердил, но у патча в текущем виде есть блокер и он чинит не все места. Ниже разбор и предлагаемый вариант.

1. Патч ломает production-сборку

$ cd packages/core && npm run build
❯ Building(💥): Error: packages/core/core.ts(120,46): semantic error TS18048 'keyGetter' is possibly 'undefined'.

Собираются только core.development.*, а core.production.min.cjs, core.production.min.mjs и core.d.ts — нет.

Причина: в scripts/rollup/build.js rollup-plugin-replace стоит до typescript2, и в production-сборке __DEV__ заменяется на false. TypeScript в недостижимом коде (if (false) { … }) отключает сужение типов, поэтому keyGetter && … перестаёт работать как type guard. Минимальное воспроизведение на tsc@4.9.4:

if (false) {
  const keyGetter = Object.getOwnPropertyDescriptor(props, 'key')?.get
  const isReactWarning = keyGetter && 'isReactWarning' in keyGetter && keyGetter.isReactWarning
}
// error TS18048: 'keyGetter' is possibly 'undefined'.
// error TS2339: Property 'isReactWarning' does not exist on type '() => any'.

Отдельно замечу, что build.js глотает ошибку и завершается с кодом 0, поэтому «сборка прошла» — это ложный сигнал.

2. Тот же баг есть в @bem-react/di

packages/di/di.tsx#L45 — {jsx(Component, props)}. Воспроизводится точно так же:

const App = withRegistry(new Registry({ id: 'registry' }))(() => null)
render(<App key="key" />)
// A props object containing a "key" prop is being spread into JSX

3. Уточнения по объёму фикса

• Ветка совпавшего модификатора (jsx(ModifiedComponent, Object.assign({}, props, { className }))) уже безопасна: React вешает key через Object.defineProperty без enumerable: true, а Object.assign неперечислимые свойства не копирует. То же касается composeSimple. Так что чинить нужно ровно два места — по одному в core и di.
• Баг актуален для всего 19.x: в свежем react@19.2.8 предупреждение о spread по-прежнему не проверяет getter.isReactWarning, хотя соседний hasValidKey в том же файле — проверяет.

4. Предлагаемый вариант

Проверку isReactWarning можно не делать: у props, полученных компонентом, собственного перечислимого key быть не может — React его вырезает. Достаточно повторить то, что делает сам jsx (if ("key" in config) { … }). Это заодно снимает проблему из п.1, потому что сужение типов больше не нужно, и не тащит __assign-хелпер при target: es5.

--- a/packages/core/core.ts
+++ b/packages/core/core.ts
@@ -113,6 +113,12 @@ export function withBemMod<T, U extends IClassNameProps = {}>(
         return jsx(ModifiedComponent, Object.assign({}, props, { className }))
       }

+      if (__DEV__) {
+        // React adds a non-enumerable `key` getter to props, passing them to `jsx`
+        // as is triggers a false "key spread" warning.
+        if ('key' in props) props = Object.assign({}, props)
+      }
+
       return jsx(WrappedComponent, props)
     }

--- a/packages/di/di.tsx
+++ b/packages/di/di.tsx
@@ -18,6 +18,12 @@ export function withRegistry() {
     const RegistryResolver: FC<P> = (props) => {
       const providedRegistriesRef = useRef<RegistryContext | null>(null)

+      if (__DEV__) {
+        // React adds a non-enumerable `key` getter to props, passing them to `jsx`
+        // as is triggers a false "key spread" warning.
+        if ('key' in props) props = Object.assign({}, props)
+      }
+
       return (
         <RegistriesConsumer>
           {(contextRegistries) => {

5. Тесты

Стоит закрыть регрессию тестами — оба падают без фикса:

// packages/core/test/withBemMod.test.tsx
test('should not warn about key spread for unmatched prop', () => {
  const spy = jest.spyOn(console, 'error').mockImplementation(() => {})
  const WBCM = withBemMod<IPresenterProps>(presenter(), { theme: 'normal' })(Presenter)

  render(<WBCM key="key" />)

  expect(spy).not.toHaveBeenCalled()
  spy.mockRestore()
})

// packages/di/test/di.test.tsx
test('should not warn about key spread', () => {
  const spy = jest.spyOn(console, 'error').mockImplementation(() => {})
  const App = withRegistry(new Registry({ id: 'registry' }))(() => null)

  render(<App key="key" />)

  expect(spy).not.toHaveBeenCalled()
  spy.mockRestore()
})

С этими изменениями: jest — 77/77, eslint — чисто, обе сборки дают все четыре бандла, в production-бандлах dev-кода не остаётся (core 1.13 kB gzip, di 826 B).

[ogolknev]

Да все валидно, спасибо. Закоммитил предложенный вариант.

[veged]

Проверил локально на актуальном master: jest 77/77, eslint чисто, core и di собирают все четыре бандла (production-бандлы больше не падают). Спасибо!