-
Notifications
You must be signed in to change notification settings - Fork 631
feat: angular signals generator improvements #1748
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: angular signals generator improvements #1748
Conversation
🦋 Changeset detectedLatest commit: 5c66772 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
View your CI Pipeline Execution ↗ for commit 5c66772.
☁️ Nx Cloud last updated this comment at |
|
@samijaber this PR is ready for review. Will create a loom shortly, going through the changes. @nmerget it would mean a lot if you could take a look too! |
samijaber
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥 🚀
| @@ -0,0 +1,11 @@ | |||
| import { BaseHook, OnMountHook } from '@/types/mitosis-component'; | |||
|
|
|||
| export const isHookEmpty = (hook?: BaseHook | BaseHook[] | OnMountHook[]): boolean => { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, been meaning to add that logic somewhere. Might even make sense to put it somewhere that applies to all generators. somewhere like
| export const initializeOptions = <T extends BaseTranspilerOptions>({ |
after processTargetBlocks runs, therefore we know if a hook is actually empty for that given target and can delete it completely.
Not for this PR, just a thought for the future
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure! yeah, saw a bunch of empty hooks inside the codebase mostly due to the usage of useTarget to add code for certain SDKs only. will create a PR for this soon
@sidmohanty11 I'm so sorry forgot to check this again. I'll update my own branch and fix some minor issues I found. I didn't check it deeply, but I think it looks very nice :) |
## Description Uses the Angular signals generator: BuilderIO/mitosis#1748 and does a major performance revamp fixes #3904 _Screenshot_ If relevant, add a screenshot or two of the changes you made.
## Description Uses the Angular signals generator: BuilderIO/mitosis#1748 and does a major performance revamp fixes BuilderIO#3904 _Screenshot_ If relevant, add a screenshot or two of the changes you made.
Description
Adds support for handling:
as Xexpressions inside template - converts them to a computed valueindexit uses just functions and if it's not then it uses Angular's computedFixes:
Make sure to follow the PR preparation steps in CONTRIBUTING.md before submitting your PR:
yarn fmt:prettier.yarn test:updateyarn g:changesetand follow the CLI instructions. Alternatively, use the Changeset Github Bot to create the file.