8000 Introduce `apply` syntax for event listeners by kubukoz · Pull Request #402 · armanbilge/calico · GitHub
[go: up one dir, main page]

Skip to content

Introduce apply syntax for event listeners - #402

Merged
armanbilge merged 9 commits into
armanbilge:mainfrom
kubukoz:onevent-syntax-apply
Oct 7, 2024
Merged

armanbilge merged 9 commits into
armanbilge:mainfrom
kubukoz:onevent-syntax-apply

Conversation

@kubukoz
@kubukoz kubukoz commented Sep 30, 2024
Copy link
Copy Markdown
Collaborator

Supersedes #223. Closes #202. Closes #400.

@kubukoz
kubukoz marked this pull request as ready for review October 1, 2024 22:45
@kubukoz
kubukoz commented Oct 1, 2024
Copy link
Copy Markdown
Collaborator Author
[error]  * static method apply(java.lang.String,scala.Function1)calico.html.EventProp in class calico.html.EventProp does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("calico.html.EventProp.apply")

I think this is about a static forwarder to this:

object EventProp:
  @inline private[html] def apply[F[_], E](key: String, f: Any => E): EventProp[F, E] =
    new EventProp(key, _.map(f))

I would assume we don't care about Java calling us so it's safe. @armanbilge please take 8000 a look boss

@armanbilge
Copy link
Copy Markdown
Owner

While I agree it's safe, I have a couple concerns:

  1. The filter is not specific to static forwarders (MiMa doesn't support this), so it could inadvertently filter an actual breaking change in the future.
  2. I don't understand how your changes caused that to happen to begin with? 🤔

@armanbilge
Copy link
Copy Markdown
Owner

2. I don't understand how your changes caused that to happen to begin with?

Bug maybe.

A workaround is to use @targetName annotation to call it something other than apply in the binary.

@kubukoz
kubukoz commented Oct 5, 2024
Copy link
Copy Markdown
Collaborator Author

Makes sense. Added targetName, seems to work fine (locally) without the exclusion.

Comment thread build.sbt Resolved
@armanbilge armanbilge left a comment
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this was overdue 😁

@armanbilge
armanbilge merged commit 60c8853 into armanbilge:main Oct 7, 2024
@armanbilge armanbilge mentioned this pull request Dec 4, 2024
@kubukoz
kubukoz deleted the onevent-syntax-apply branch December 5, 2024 02:47
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.

--> alternatives / overloads onClick --> { _.foreach { _ => ... } } is burdensome

2 participants

0