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

Accommodate annotation on Props class #476

Merged
merged 1 commit into from
May 18, 2021

Conversation

jedahu
Copy link
Contributor

@jedahu jedahu commented May 13, 2021

Given this component definition:

@react
object Foo {
  @something
  case class Props(a: Int, b: String)

  val component = FunctionalComponent[Props} { ... }
}

The react macro doesn't match case class Props because of the something annotation, and generates,

  def apply(props: Props)

instead of the expected,

  def apply[..$tparams](...$paramssWithoutChildren)...

This change fixes this, but only for functional component definitions, because they are the only ones I have tested so far.

@shadaj
Copy link
Owner

shadaj commented May 14, 2021

Thanks for the PR @jedahu! The changes look good to me, but looks like you'll have to run scalafmt once.

Given this component definition:

```scala
@react
object Foo {
  @something
  case class Props(a: Int, b: String)

  val component = FunctionalComponent[Props} { ... }
}
```

The `react` macro doesn't match `case class Props` because of the `something`
annotation, and generates,

```scala
  def apply(props: Props)
```

instead of the expected,

```scala
  def apply[..$tparams](...$paramssWithoutChildren)...
```

This change fixes this, but only for functional component definitions, because
they are the only ones I have tested so far.
@jedahu jedahu force-pushed the accommodate-props-annotations branch from 3376ad2 to 041aca7 Compare May 16, 2021 03:35
@shadaj shadaj merged commit 6f1eb7d into shadaj:master May 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants