-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
refactor(core): simplify usage of app event and window label types #1623
Conversation
0bfd525 changes all the interfaces that might not need an owned tag into a new trait, If an item is going to be stored (like an Event or Label tag in an event Listener), we accept a I think that commit covers all the event stuff, I'll double check to make sure there's no window labels lying around in the API that need the same treatment. |
Looks like This PR is probably good once the tests pass. I don't think this is actually a breaking change, because it previously accepted edit: nope, |
@chippers any chance we can also accept owned values where I tried and it seems like it's not easy or even possible :/ |
I'll look into it tonight or tomorrow. It would look less weird if the event or label was already stored in a variable. I chose to take the reference because those items don't necessarily clone, so this is the only way to absolutely prevent an allocation by the end user. imo immediately referencing a newly created value isn't too weird anyways. I'll still take a look at possibilities tonight/tomorrow |
alright, don't waste too much time on it :P |
In short, we can allow
Since allowing let map: HashMap<MyEvent, String> = HashMap::new();
map.get(&MyEvent::Foo); is similar to window.emit(&MyEvent::Foo, None); This PR is ready if there are no more reviews |
What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
The PR fulfills these requirements:
fix: #xxx[,#xxx]
, where "xxx" is the issue number)If adding a new feature, the PR's description includes:
Other information: