Skip to content

Introduce apply syntax for event listeners#402

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

Introduce apply syntax for event listeners#402
armanbilge merged 9 commits intoarmanbilge:mainfrom
kubukoz:onevent-syntax-apply

Conversation

@kubukoz
Copy link
Copy Markdown
Collaborator

@kubukoz kubukoz commented Sep 30, 2024

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

@kubukoz kubukoz marked this pull request as ready for review October 1, 2024 22:45
@kubukoz
Copy link
Copy Markdown
Collaborator Author

kubukoz commented Oct 1, 2024

[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 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
Copy link
Copy Markdown
Collaborator Author

kubukoz commented Oct 5, 2024

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

Comment thread build.sbt Outdated
Copy link
Copy Markdown
Owner

@armanbilge armanbilge left a comment

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