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

refactor(events): Introduce and use event type predicates #8538

Merged
merged 4 commits into from
Aug 20, 2024

Conversation

cpcallen
Copy link
Contributor

The basics

The details

Proposed Changes

  • Introduce predicates for testing Abstract event subclasses based on their .type properties. These are useful because there are places where it is not possible to use instanceof <ClassConstructor> tests for type narrowing due to load ordering issues that would be caused by the need to import (rather than just import type) the class
    constructors in question.

  • Simplify several sections of code by using these type predicates for type narrowing, and thereby avoiding the need for explicit casts.

Reason for Changes

Cleanup and preparation for fixing #8225 et al.

Test Coverage

  • Passes npm test.
  • Trashcan behaviour has been tested manually to ensure refactor has not broken anything.

Additional Information

The type predicates are not currently reexported from core/blockly.ts. This could be done by adding suitable export … from … statements to core/events/events.ts, but I've opted not to do this until there is a good reason to: in most cases it is probably preferable that code use e instanceof EventClass rather than isEventClass(e), since the former is a stricter and less easily subverted type check, and code outside of core/ can always access the event constructors without any cyclic import issues.

Sorry, something went wrong.

Introduce predicates for testing Abstract event subclasses based on
their .type properties.  These are useful because there are places
where it is not possible to use instanceof <ClassConstructor> tests
for type narrowing due to load ordering issues that would be caused
by the need to import (rather than just import type) the class
constructors in question.
Simplify several sections of code by using type predicates for
type narrowing and thereby avoiding the need for explicit casts.
@cpcallen cpcallen force-pushed the refactor/event-type-predicates branch from 0f6ac10 to 0b60195 Compare August 20, 2024 12:38
Copy link
Collaborator

@rachel-fenichel rachel-fenichel left a comment

Choose a reason for hiding this comment

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

lgtm once you fix lint and formatting

core/events/predicates.ts Outdated Show resolved Hide resolved

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@cpcallen
Copy link
Contributor Author

lgtm once you fix lint and formatting

Yeah: since I set up auto-sorting of imports, and this by default removes unused imports, any remaining unused imports now trigger both lint and format failures.

@cpcallen cpcallen merged commit 032b5ed into google:develop Aug 20, 2024
7 checks passed
@cpcallen cpcallen deleted the refactor/event-type-predicates branch August 20, 2024 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants