Skip to content
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

JSX parsing panics when using the spread operator #2037

Closed
kitsonk opened this issue Aug 9, 2021 · 17 comments · Fixed by #6505
Closed

JSX parsing panics when using the spread operator #2037

kitsonk opened this issue Aug 9, 2021 · 17 comments · Fixed by #6505
Assignees
Labels
Milestone

Comments

@kitsonk
Copy link
Contributor

kitsonk commented Aug 9, 2021

Describe the bug

Using the spread operator in a JSX block causes a panic (at least in Deno).

Input code

const A = () => {
  return <div>{...[]}</div>;
};

Expected behavior

The code to be emitted without a panic and something like:

"use strict";
const A = () => {
    return React.createElement("div", null, []);
};

Actual behaviour

❯ RUST_BACKTRACE=1 deno bundle --no-check spread.tsx
Bundle file:///Users/kitsonk/github/deploy_dg/spread.tsx
thread 'main' panicked at 'cannot access a scoped thread local variable without calling `set` first', /Users/kitsonk/.cargo/registry/src/github.com-1ecc6299db9ec823/scoped-tls-1.0.0/src/lib.rs:168:9
stack backtrace:
   0: std::panicking::begin_panic
   1: swc_ecma_transforms_react::jsx::Jsx<C>::jsx_elem_child_to_expr
   2: core::iter::traits::iterator::Iterator::find_map::check::{{closure}}
   3: <core::iter::adapters::chain::Chain<A,B> as core::iter::traits::iterator::Iterator>::next
   4: <alloc::vec::Vec<T,A> as alloc::vec::spec_extend::SpecExtend<T,I>>::spec_extend
   5: swc_ecma_transforms_react::jsx::Jsx<C>::jsx_elem_to_expr
   6: <swc_ecma_transforms_react::jsx::Jsx<C> as swc_ecma_visit::VisitMut>::visit_mut_expr
   7: swc_ecma_visit::visit_mut_stmts
   8: <swc_ecma_transforms_react::jsx::Jsx<C> as swc_ecma_visit::VisitMut>::visit_mut_expr
   9: swc_ecma_visit::visit_mut_var_declarators
  10: <swc_visit::Optional<V> as swc_ecma_visit::Fold>::fold_module
  11: swc_bundler::bundler::load::<impl swc_bundler::bundler::Bundler<L,R>>::load_transformed
  12: <core::iter::adapters::map::Map<I,F> as core::iter::traits::iterator::Iterator>::next
  13: deno::module_graph::Graph::emit_bundle
  14: deno::bundle_module_graph
  15: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
  16: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
  17: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
  18: deno::main

Version

swc_common = "0.11.4"
swc_ecmascript = "0.46.0"

Additional context

@kdy1
Copy link
Member

kdy1 commented Aug 9, 2021

Seems like swc_ecma_utils::HANDLER is not configured?

@kdy1
Copy link
Member

kdy1 commented Aug 9, 2021

Btw, it does not look like an erroneous input.

@kdy1 kdy1 modified the milestones: v1.2.76, v1.2.77 Aug 9, 2021
kdy1 added a commit to kdy1/swc that referenced this issue Aug 9, 2021
@NamPNQ
Copy link

NamPNQ commented Aug 10, 2021

I think the expected behavior is to raise an error

For information, babel it will raise errors:
Spread children are not supported in React.

@kitsonk
Copy link
Contributor Author

kitsonk commented Aug 10, 2021

@NamPNQ TypeScript transpiles it without error. Also there is a big difference between an error and a panic.

@kdy1
Copy link
Member

kdy1 commented Aug 11, 2021

@kitsonk
It panics because swc_ecma_utils::HANDLER is not configured

@kdy1 kdy1 removed this from the v1.2.77 milestone Aug 11, 2021
@kitsonk
Copy link
Contributor Author

kitsonk commented Aug 11, 2021

Ok, I will look into that. It seems we don't have that set across several projects.

The input isn't an error though, as I mentioned, TypeScript transforms it without error.

@kdy1
Copy link
Member

kdy1 commented Aug 11, 2021

swc also transpiles it without an error.
I'm not sure if it's related to HANDLER.

@kdy1
Copy link
Member

kdy1 commented Aug 21, 2021

Can you try configuring swc_ecma_utils::HANDLER?

@kitsonk
Copy link
Contributor Author

kitsonk commented Aug 23, 2021

I am trying to, but we have never used HANDLER.set() anywhere in Deno before. I think I have figured out what I need to do to create a handler, but I am not exactly sure where it needs to be scoped in our code.

Specifically when we bundle, we first create a swc_bundler::Bundler and then take the .bundle() and run it through a swc_ecmascript::codegen::Emitter. Where does the scoping of the HANDLER.set() need to occur?

@kdy1
Copy link
Member

kdy1 commented Aug 23, 2021

I think scope should be the invokation ofjsx transform.
I'm not sure if it will fix the issue. I asked trying it because I can't guess a cause of panic.

@dsherret
Copy link
Contributor

dsherret commented Nov 15, 2021

It was because swc_ecma_utils::HANDLER wasn't set. This API seems very hidden and I'm not sure ideal (ex. typescript's api returns emit diagnostics as an array on a property of the emit result). I implemented it in Deno in denoland/deno#12773 by collecting the diagnostics and then returning an error after transforming if a diagnostic with a certain level appeared. Works ok, I think, now that we know about it.

The error we get now is: Spread children are not supported in React.. I see Babel gives that error, but as Kitson pointed out TypeScript seems to handle this transform. I don't think there's any harm in implementing it.

@kdy1
Copy link
Member

kdy1 commented Nov 15, 2021

I'll do so, as I prefer behavior of tsc of it of babel.

@binoche9
Copy link

binoche9 commented Nov 9, 2022

How difficult would the fix for this be (if someone with no context of the swc codebase would try to fix this)?

@kdy1
Copy link
Member

kdy1 commented Nov 10, 2022

It's not hard, but not sure how much it will be for others

@Hotell

This comment has been minimized.

@kdy1 kdy1 added this to the Planned milestone Nov 24, 2022
@kdy1 kdy1 self-assigned this Nov 24, 2022
kdy1 added a commit that referenced this issue Nov 24, 2022
@Hotell

This comment has been minimized.

@kdy1 kdy1 modified the milestones: Planned, v1.3.20 Nov 26, 2022
@swc-bot
Copy link
Collaborator

swc-bot commented Dec 27, 2022

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@swc-project swc-project locked as resolved and limited conversation to collaborators Dec 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging a pull request may close this issue.

7 participants