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

Remove Evented mixin. Use native EventTarget. #2875

Merged
merged 3 commits into from
Dec 25, 2024

Conversation

BobrImperator
Copy link
Collaborator

@BobrImperator BobrImperator commented Dec 25, 2024

  • Removes Evented Mixin
    This is a prerequisite to making everything ES6 classes and make it easier to convert to Typescript.

Mostly keeps the API the same except for the fact that we extend the Native EventTarget and emit CustomEvents which require to provide data as a detail field.

Event handlers must change from

  authenticator.on('sessionDataUpdated', data => {

to

 authenticator.on('sessionDataUpdated', ({ detail: data }) => {

Additionally, not the whole Evented API is implemented on the Authenticator, Store and InternalSession objects.
Only on, off and trigger methods are supported.

Currently I'm not convinced that the events are needed but also don't see a need to refactor away.

Lastly, I've discovered a different behavior of the internal-session.js once the ObjectProxy was refactored to ES6 such as export default class InternalSession extends ObjectProxy {} as opposed to the current classic syntax export default ObjectProxy.extend({}), for some reason set,get and setProperties calls proxying misbehave when the methods are implemented outside the extend({}) body.

Copy link

github-actions bot commented Dec 25, 2024

Some tests with 'continue-on-error: true' have failed:

  • test-esa test:one ember-beta

  • test-app test:one ember-canary

  • classic-test-app test:one ember-beta

  • test-app test:one ember-beta

  • test-esa test:one ember-canary

    Created by continue-on-error-comment

Copy link

Some tests with 'continue-on-error: true' have failed:

  • classic-test-app test:one ember-canary

Created by continue-on-error-comment

@BobrImperator BobrImperator merged commit 4ed6d5b into master Dec 25, 2024
39 of 42 checks passed
@BobrImperator BobrImperator deleted the remove-evented-mixin branch December 25, 2024 01:06
@github-actions github-actions bot mentioned this pull request Dec 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant