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

feat: support ControllerExtensions extended with .override(...) #128

Merged
merged 4 commits into from
Jul 12, 2024

Conversation

akudev
Copy link
Contributor

@akudev akudev commented Jun 24, 2024

Recognize (and remove) a dummy function ControllerExtension.use(...)
that only exists on TypeScript/ES-class side and takes care of
converting the class resulting from SomeExtension.override(...) to the
instance type that needs to be presented to TypeScript for the member
property.

Intentionally does not remove the previous @transformControllerExtension
annotation to not disrupt potential users immediately. But it will be
removed.

Contains some unrelated whitespace fixes for prettier.

@akudev akudev requested a review from petermuessig June 24, 2024 13:05
@akudev akudev force-pushed the feat/controllerextensions branch from 0c5138f to ba68506 Compare June 26, 2024 10:01
@akudev akudev requested a review from codeworrior June 26, 2024 10:02
Recognize (and remove) a dummy function ControllerExtension.use(...)
that only exists on TypeScript/ES-class side and takes care of
converting the class resulting from SomeExtension.override(...) to the
instance type that needs to be presented to TypeScript for the member
property.

Intentionally does not remove the previous @transformControllerExtension
annotation to not disrupt potential users immediately. But it will be
removed.

Contains some unrelated whitespace fixes for prettier.
@akudev akudev force-pushed the feat/controllerextensions branch from ba68506 to 399b507 Compare July 4, 2024 10:28
@akudev akudev marked this pull request as ready for review July 4, 2024 10:29
petermuessig
petermuessig previously approved these changes Jul 11, 2024
Copy link
Member

@petermuessig petermuessig left a comment

Choose a reason for hiding this comment

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

Bascially looks good - just if there could be somewhere an undefined when using the objects? But if this is no problem from your side => APPROVED!

packages/plugin/src/classes/helpers/classes.js Outdated Show resolved Hide resolved
Throw error when ControllerExtension.use(...) is not called with one
parameter.
@akudev
Copy link
Contributor Author

akudev commented Jul 12, 2024

Ummm... I think the test error is because the error message is not colored but expected to be colored:
image

@akudev akudev force-pushed the feat/controllerextensions branch from 66cba1e to 3777d89 Compare July 12, 2024 12:04
@akudev akudev merged commit f744d9d into main Jul 12, 2024
6 checks passed
@akudev akudev deleted the feat/controllerextensions branch July 12, 2024 12:08
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.

2 participants