revamp angular generator #1457
revamp angular generator #1457
Conversation
🦋 Changeset detectedLatest commit: b95a6d1 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 |
☁️ Nx Cloud ReportCI is running/has finished running commands for commit b95a6d1. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 4 targetsSent with 💌 from NxCloud. |
sidmohanty11
force-pushed
the
fix/revamp-angular-generator
branch
from
f89992d
to
fd31ae2
Compare
| @@ -290,6 +229,7 @@ const handleNgOutletBindings = (node: MitosisNode) => { | |||
| }; | |||
|
|
|||
| export const blockToAngular = ( | |||
| root: MitosisComponent, | |||
There was a problem hiding this comment.
can we change blockToAngular to use named arguments?
export const blockToAngular = ({ root, json, options, blockOptions }: {/**/}) => {}
VSCode can usually do that for you with one click:
CleanShot.2024-06-06.at.17.14.02.mp4
There was a problem hiding this comment.
thanks a lot! didn't know this was that simple
| const inputsPropsStateName = `mergedInputs_${hashCodeAsString(allProps)}`; | ||
| root.state[inputsPropsStateName] = { | ||
| code: 'null', | ||
| type: 'property', | ||
| }; | ||
| if ( | ||
| !root.hooks.onMount | ||
| .map((hook) => hook.code) | ||
| .join('') | ||
| .includes(inputsPropsStateName) | ||
| ) { | ||
| root.hooks.onMount.push({ | ||
| code: `this.${inputsPropsStateName} = {${allProps}}`, | ||
| onSSR: false, | ||
| }); | ||
| } | ||
| if ( | ||
| root.hooks.onUpdate && | ||
| root.hooks.onUpdate.length > 0 && | ||
| !root.hooks.onUpdate | ||
| .map((hook) => hook.code) | ||
| .join('') | ||
| .includes(inputsPropsStateName) | ||
| ) { | ||
| root.hooks.onUpdate.push({ | ||
| code: `this.${inputsPropsStateName} = {${allProps}}`, | ||
| }); | ||
| } |
There was a problem hiding this comment.
why do we need this workaround for ngComponentOutlet's inputs?
There was a problem hiding this comment.
Ah just like the other bindings: to avoid the template issues with JS expressions
There was a problem hiding this comment.
yes plus previously we used to send wrapperProps as a named prop for spread props..
for example,
import { Component, ChangeDetectorRef, Input } from '@angular/core';
import {
RegisteredComponent,
fetchOneEntry,
type BuilderContent,
} from '@builder.io/sdk-angular';
@Component({
selector: 'hello',
template: `hello!asfsa - {{ builderProps.text }}`,
})
export class HelloComponent {
@Input() text!: string;
@Input('wrapperProps') builderProps!: any;
}
here we need to get text but instead we were getting text from builderProps (builderProps.text) and manually attaching them onInit. But with this we are handling it outside so, whatever attributes are spread there it should be directly attached. This will also help us attaching inputs directly from devTools (which @harmeet-builder is blocked on)
| @@ -796,7 +1029,7 @@ export const componentToAngular: TranspilerGenerator<ToAngularOptions> = | |||
| ${ | |||
| !json.hooks.onUpdate?.length | |||
| ? '' | |||
| : `ngAfterContentChecked() { | |||
| : `ngOnChanges() { | |||
There was a problem hiding this comment.
@sidmohanty11 is there a reason why you've changed this?
It's a breaking change for our project :S
There was a problem hiding this comment.
@nmerget apologies for that. We needed a way for our SDKs to listen to all updates related to that component, which is why we made this change. Let me get in touch with @samijaber regarding this and if we can keep this behind a flag so that we can use either option
There was a problem hiding this comment.
@nmerget can you expand on why this change broke your project?
If possible, create a github issue outlining how the behaviour worked before, and why it no longer works with ngOnChanges.
That will help us find the best path forward for all Mitosis users
Description
Major revamp to our angular generator, contains:
Forclass-propertiesJira
https://builder-io.atlassian.net/browse/ENG-5866
Make sure to follow the PR preparation steps in CONTRIBUTING.md before submitting your PR: