remix icon indicating copy to clipboard operation
remix copied to clipboard

Full app crash if prop spread is before key inside loop

Open Swapnull opened this issue 1 year ago • 10 comments

What version of Remix are you using?

1.7.0

Steps to Reproduce

Reproduction is available: https://github.com/Swapnull/remix-spread-key-error/blob/main/app/routes/index.tsx

After upgrading from 1.6.5 to 1.7.0, When calling a component in a map while spreading props, the spread must be after the key, otherwise a Element type is invalid: expected a string... error will happen.

For example

const spreadable = { text: "Hi!" }; 
return myArray.map((item, index) => <SomeComponent {...spreadable} key={index} />) {/* Causes error */}
return myArray.map((item, index) => <SomeComponent key={index}  {...spreadable}  />) {/* Does not cause error */}
return <SomeComponent {...spreadable} key={index} /> {/* Does not cause error */}

Video of me demonstrating the issue: https://www.loom.com/share/1b8923577ae84e1e93f41be75b12cf77

Expected Behavior

It should not matter what order props appear in.

Actual Behavior

My large (150 route) remix app is breaking when upgrading to 1.7.0

Swapnull avatar Aug 26 '22 18:08 Swapnull

I had the same problem in 1.7.0, I observed conflicting import symbols in the build target, and for now I had to rewrite the import to get around the error.

source:

// b.tsx
import { RemixServer } from '@remix-run/react'

target:

// a.tsx
var import_react = require('react'), ...
      ^^^

//  b.tsx
var import_react = require('@remix-run/react'), ...
      ^^^
// c.tsx
var import_react2 = require('react'), ...

Bypass this problem:

// b.tsx
const { RemixServer } = require('@remix-run/react')

FlatMapIO avatar Aug 28 '22 01:08 FlatMapIO

Thanks for the workaround @FlatMapIO also experienced this when upgrading a grunge stack to v 1.7

Xiphe avatar Aug 29 '22 12:08 Xiphe

can confirm, I had a few spreads before the key was defined, moving the key to the start of the element fixed this error for me

<li {...props} key={option.id}>
 {option.dropDownName}
</li>

vs

<li key={option.id} {...props}>
 {option.dropDownName}
</li>

JeffBeltran avatar Aug 30 '22 14:08 JeffBeltran

@JeffBeltran thanks for the workaround, it worked here too

franklinjavier avatar Aug 30 '22 20:08 franklinjavier

I can also confirm the issue and the workaround worked.

sapter avatar Sep 01 '22 11:09 sapter

I got a random error updating patch-version packages. Turns out this was the culprit. Thanks!

darenmalfait avatar Sep 07 '22 13:09 darenmalfait

Seems to have been fixed upstream in esbuild https://github.com/evanw/esbuild/issues/2534 Fix will probably land in esbuild 0.15.8

machour avatar Sep 09 '22 13:09 machour

Wow, gnarly bug. Just ran into this. What made it especially difficult is that the crash occurs at entry.server.tsx:19 and not at the component that generates the name collision. Super rare bug. Looking forward to this fix, thanks.

divmgl avatar Oct 05 '22 05:10 divmgl

Also just ran into this issue. Weird one for sure.

wking-io avatar Oct 26 '22 13:10 wking-io

Also run into that bug today. Here is a repo with a react-vite and remix-app for comparison. https://github.com/tobiasfoerg/remix-key-spread-bug

tobiasfoerg avatar Nov 10 '22 15:11 tobiasfoerg