-
Notifications
You must be signed in to change notification settings - Fork 143
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Spread children not supported #420
Comments
Interesting, I actually hadn't seen that syntax. FWIW, Babel (with the React transform) throws an exception, saying "Spread children are not supported in React" (repl). Are you using something other than React? I think React wants you to pass an array there so it knows to check that you're using Regardless, I think it's within Sucrase's scope to just compile it and let other tools do the error checking, so this certainly seems reasonable to implement. |
Yeah I'm using React. FWIW this is syntax that Typescript supported. My
understanding of this syntax is that it's for places where you have an
array but know it's actually static and unchanging, so that keys are not
necessary (so React can spread them out as if they were positional args to
createElement). It's also useful when you're transplanting `children` from
one place to another.
I don't know why Babel chokes on this, though I've seen a higher rate of
bugs in Babel than TSC so far since the switch in create react app 2.0
([random example](babel/babel#9385)).
Thanks!
…On Fri, Feb 22, 2019 at 7:14 AM Alan Pierce ***@***.***> wrote:
Interesting, I actually hadn't seen that syntax. FWIW, Babel (with the
React transform) throws an exception, saying "Spread children are not
supported in React" (repl
<https://babeljs.io/repl#?babili=false&browsers=&build=&builtIns=false&spec=false&loose=false&code_lz=FAYw9gdgzgLgBAMTgXjgCgJQoHxwDwCG2A3gHTkDaAugL54D0RA3EA&debug=false&forceAllTransforms=false&shippedProposals=false&circleciRepo=&evaluate=false&fileSize=false&timeTravel=false&sourceType=module&lineWrap=true&presets=es2015%2Creact%2Cstage-2&prettier=false&targets=&version=7.3.3>).
Are you using something other than React? I think React wants you to pass
an array there so it knows to check that you're using keys properly.
Regardless, I think it's within Sucrase's scope to just compile it and let
other tools do the error checking, so this certainly seems reasonable to
implement.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#420 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAb2V-Vv--U8HfQQBqxWfTL8e9voKBEks5vQAlPgaJpZM4bGMwh>
.
|
https://facebook.github.io/jsx/ includes it:
|
Some history here: facebook/jsx#57 . Looks like the original argument for adding it was for non-React use cases. My impression is Babel parses and recognizes it but intentionally disallows it for React, since they're worried that beginners will use it for dynamic arrays when really they should be specifying keys. TypeScript didn't implement that check, it seems. But I think that restriction is something a linter should do (and a team should be allowed to opt in and out of), so I think it's fine to just allow and transform the syntax as specified in Sucrase. |
@yang to be clear, you expect <div>{...a}{...b}</div> to be transformed to React.createElement('div', null, ...a, ...b) right? That's certainly what I'd expect. Looks like TypeScript is transforming it to I'll implement it with |
Fixes #420 This was actually already handled by the parser carried over from Babel, but there was a bug in the tokenization, since Sucrase always skips the last token so we can get the proper token type for the next one. After fixing that bug, everything seems to work; no transformer changes are needed because argument spread syntax is the same as JSX child spread syntax. As mentioned in #420, Babel disallows this syntax with React and TypeScript seems to give the wrong result, but this change like a good idea regardless since it's just a bug fix.
Fixes #420 This was actually already handled by the parser carried over from Babel, but there was a bug in the tokenization, since Sucrase always skips the last token so we can get the proper token type for the next one. After fixing that bug, everything seems to work; no transformer changes are needed because argument spread syntax is the same as JSX child spread syntax. As mentioned in #420, Babel disallows this syntax with React and TypeScript seems to give the wrong result, but this change like a good idea regardless since it's just a bug fix.
I just released a fix for this in 3.10.0. |
crashes sucrase with:
The text was updated successfully, but these errors were encountered: