-
Notifications
You must be signed in to change notification settings - Fork 589
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
Fix broken angular block #1705
base: main
Are you sure you want to change the base?
Fix broken angular block #1705
Conversation
🦋 Changeset detectedLatest commit: 81e48f0 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 81e48f0.
☁️ Nx Cloud last updated this comment at |
|
||
// The generated Angular code should not have nested double quotes | ||
expect(template).not.toMatch(/="[^"]*"[^"]*"/); | ||
}); |
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.
in general, we have the approach of using expect(template).toMatchInlineSnapshot()
so that we can notice any surprising changes across all tests when new PRs are made.
If you don't mind adding inline snapshots for relevant objects. See https://github.com/BuilderIO/mitosis/blob/main/packages/core/src/__tests__/builder.test.ts for examples of how we use it.
oyou just write toMatchInlineSnapshot()
, and once you run the tests it will populate it. See https://vitest.dev/guide/snapshot#inline-snapshots
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.
@samijaber thanks for the suggestion, updated it
str += ` [innerHTML]="sanitizer.bypassSecurityTrustHtml('${safeValue}')" `; | ||
} else { | ||
// For standard attribute (though this won't work in Angular) | ||
str += ` innerHTML="${safeValue}" `; |
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.
we still need to convert this to [innerHTML]="'${safeValue}'"
right?
import {Component} from '@angular/core';
import {bootstrapApplication} from '@angular/platform-browser';
@Component({
selector: 'app-root',
template: `
<div [innerHTML]="'hello "'">
</div>
`,
})
export class PlaygroundComponent {}
bootstrapApplication(PlaygroundComponent);
innerHTML="text" doesn't work in https://angular.dev/playground
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.
Last bit, can we please add a test for this? From Mitosis -> Angular generation?
For example, we can use this as the *.raw.tsx
export default function MyComponent(props) {
return (
<div innerHTML='hello "'>
Hello! I can run natively in React, Vue, Svelte, Qwik, and many more frameworks!
</div>
);
}
this should convert the quote and update the innerHTML
to [innerHTML]
To do this,
- we can add a new raw file here: https://github.com/BuilderIO/mitosis/tree/main/packages/core/src/__tests__/data/angular
- include this test here: https://github.com/BuilderIO/mitosis/blob/main/packages/core/src/__tests__/test-generator.ts#L366
- update the snapshots
Description
Updated parsing of innerHtml which was causing issues on code gen for angular.
yarn fmt:prettier
.yarn test:update
yarn g:changeset
and follow the CLI instructions. Alternatively, use the Changeset Github Bot to create the file.