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.

Sign up for GitHub

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/comments with quotes #1320

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Fix/comments with quotes #1320

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
60583ef
test: add test to test generator
SamiKamal Dec 17, 2023
b287044
fix: clear comments in `parseCode`
SamiKamal Dec 17, 2023
088402a
refactor: update arg name
SamiKamal Dec 17, 2023
6768d20
update snapshots
SamiKamal Jan 2, 2024
8b93068
lint
SamiKamal Jan 2, 2024
d3241f4
revert settings.json change
SamiKamal Jan 2, 2024
2ba4dbd
fix typo
SamiKamal Jan 2, 2024
a3c9ec3
remove space
SamiKamal Jan 2, 2024
1573747
Merge branch 'main' into fix/comments-with-quotes
SamiKamal Jan 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
179 changes: 82 additions & 97 deletions packages/core/src/__tests__/__snapshots__/alpine.test.ts.snap
View file
Open in desktop

Large diffs are not rendered by default.

122 changes: 20 additions & 102 deletions packages/core/src/__tests__/__snapshots__/angular.import.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -843,9 +843,7 @@ export class CustomCode {
scriptsInserted = [];
scriptsRun = [];
findAndRunScripts() {
// TODO: Move this function to standalone one in '@builder.io/utils'
if (this.elem.nativeElement && typeof window !== \\"undefined\\") {
/** @type {HTMLScriptElement[]} */
const scripts = this.elem.nativeElement.getElementsByTagName(\\"script\\");

for (let i = 0; i < scripts.length; i++) {
Expand Down Expand Up @@ -923,9 +921,7 @@ export class CustomCode {
scriptsInserted = [];
scriptsRun = [];
findAndRunScripts() {
// TODO: Move this function to standalone one in '@builder.io/utils'
if (this.elem.nativeElement && typeof window !== \\"undefined\\") {
/** @type {HTMLScriptElement[]} */
const scripts = this.elem.nativeElement.getElementsByTagName(\\"script\\");

for (let i = 0; i < scripts.length; i++) {
Expand Down Expand Up @@ -1101,8 +1097,7 @@ export class FormComponent {
const el = event.currentTarget;
const headers = this.customHeaders || {};
let body;
const formData = new FormData(el); // TODO: maybe support null

const formData = new FormData(el);
const formPairs = Array.from(
event.currentTarget.querySelectorAll(\\"input,select,textarea\\")
)
Expand All @@ -1129,7 +1124,6 @@ export class FormComponent {
value = num;
}
} else if (el.type === \\"file\\") {
// TODO: one vs multiple files
value = el.files;
} else {
value = el.value;
Expand Down Expand Up @@ -1157,14 +1151,11 @@ export class FormComponent {
) {
contentType = \\"multipart/form-data\\";
}
}); // TODO: send as urlEncoded or multipart by default
// because of ease of use and reliability in browser API
// for encoding the form?
});

if (contentType !== \\"application/json\\") {
body = formData;
} else {
// Json
const json = {};
Array.from(formPairs).forEach(({ value, key }) => {
set(json, key, value);
Expand All @@ -1173,10 +1164,7 @@ export class FormComponent {
}

if (contentType && contentType !== \\"multipart/form-data\\") {
if (
/* Zapier doesn't allow content-type header to be sent from browsers */
!(sendWithJs && this.action?.includes(\\"zapier.com\\"))
) {
if (!(sendWithJs && this.action?.includes(\\"zapier.com\\"))) {
headers[\\"content-type\\"] = contentType;
}
}
Expand All @@ -1201,15 +1189,11 @@ export class FormComponent {
}/api/v1/form-submit?apiKey=\${builder.apiKey}&to=\${btoa(
this.sendSubmissionsToEmail || \\"\\"
)}&name=\${encodeURIComponent(this.name || \\"\\")}\`;
fetch(
this.sendSubmissionsTo === \\"email\\" ? formUrl : this.action,
/* TODO: throw error if no action URL */
{
body,
headers,
method: this.method || \\"post\\",
}
).then(
fetch(this.sendSubmissionsTo === \\"email\\" ? formUrl : this.action, {
body,
headers,
method: this.method || \\"post\\",
}).then(
async (res) => {
let body;
const contentType = res.headers.get(\\"content-type\\");
Expand All @@ -1221,13 +1205,10 @@ export class FormComponent {
}

if (!res.ok && this.errorMessagePath) {
/* TODO: allow supplying an error formatter function */
let message = get(body, this.errorMessagePath);

if (message) {
if (typeof message !== \\"string\\") {
/* TODO: ideally convert json to yaml so it woul dbe like
error: - email has been taken */
message = JSON.stringify(message);
}

Expand All @@ -1252,13 +1233,11 @@ export class FormComponent {
if (submitSuccessEvent.defaultPrevented) {
return;
}
/* TODO: option to turn this on/off? */

if (this.resetFormOnSubmit !== false) {
this.formRef.nativeElement.reset();
}
}
/* TODO: client side route event first that can be preventDefaulted */

if (this.successUrl) {
if (this.formRef.nativeElement) {
Expand Down Expand Up @@ -1366,7 +1345,6 @@ export class Image {
this.imageLoaded = true;
}
useLazyLoading() {
// TODO: Add more checks here, like testing for real web browsers
return !!this.lazy && this.isBrowser();
}
isBrowser = function isBrowser() {
Expand Down Expand Up @@ -2433,6 +2411,8 @@ export class MyBasicComponentModule {}
"
`;

exports[`Angular with Preserve Imports and File Extensions > jsx > Javascript Test > commentsWithQuotes 1`] = `"JSON5: invalid character '&' at 2:10"`;

exports[`Angular with Preserve Imports and File Extensions > jsx > Javascript Test > componentWithContext 1`] = `
"import { NgModule } from \\"@angular/core\\";
import { CommonModule } from \\"@angular/common\\";
Expand Down Expand Up @@ -3453,7 +3433,6 @@ export class RenderBlock {
const ref = this.context.registeredComponents[componentName];

if (!ref) {
// TODO: Public doc page with more info about this message
console.warn(\`
Could not find a registered this.component named \\"\${componentName}\\".
If you registered it, is the file that registered it imported by the file that needs to render it?\`);
Expand Down Expand Up @@ -3506,11 +3485,6 @@ export class RenderBlock {
componentRef: this.component?.component,
componentOptions: {
...getBlockComponentOptions(this.useBlock),

/**
* These this.attributes are passed to the wrapper element when there is one. If \`noWrap\` is set to true, then
* they are provided to the this.component itself directly.
*/
...(this.shouldWrap
? {}
: {
Expand All @@ -3522,28 +3496,14 @@ export class RenderBlock {
};
}
get useChildren() {
// TO-DO: When should \`canHaveChildren\` dictate rendering?
// This is currently commented out because some Builder components (e.g. Box) do not have \`canHaveChildren: true\`,
// but still receive and need to render children.
// return state.componentInfo?.canHaveChildren ? state.useBlock.children : [];
return this.useBlock.children ?? [];
}
get childrenWithoutParentComponent() {
/**
* When there is no \`componentRef\`, there might still be children that need to be rendered. In this case,
* we render them outside of \`componentRef\`.
* NOTE: We make sure not to render this if \`repeatItemData\` is non-null, because that means we are rendering an array of
* blocks, and the children will be repeated within those blocks.
*/
const shouldRenderChildrenOutsideRef =
!this.component?.component && !this.repeatItemData;
return shouldRenderChildrenOutsideRef ? this.useChildren : [];
}
get repeatItemData() {
/**
* we don't use \`state.useBlock\` here because the processing done within its logic includes evaluating the block's bindings,
* which will not work if there is a repeat.
*/
const { repeat, ...blockWithoutRepeat } = this.block;

if (!repeat?.collection) {
Expand Down Expand Up @@ -3604,7 +3564,6 @@ export class RenderBlock {
if (TARGET === \\"reactNative\\") {
return RenderComponentWithContext;
} else if (TARGET === \\"vue3\\") {
// vue3 expects a string for the this.component tag
return \\"RenderComponent\\";
} else {
return RenderComponent;
Expand Down Expand Up @@ -5177,9 +5136,7 @@ export class CustomCode {
scriptsInserted = [];
scriptsRun = [];
findAndRunScripts() {
// TODO: Move this function to standalone one in '@builder.io/utils'
if (this.elem.nativeElement && typeof window !== \\"undefined\\") {
/** @type {HTMLScriptElement[]} */
const scripts = this.elem.nativeElement.getElementsByTagName(\\"script\\");

for (let i = 0; i < scripts.length; i++) {
Expand Down Expand Up @@ -5262,9 +5219,7 @@ export class CustomCode {
scriptsInserted = [];
scriptsRun = [];
findAndRunScripts() {
// TODO: Move this function to standalone one in '@builder.io/utils'
if (this.elem.nativeElement && typeof window !== \\"undefined\\") {
/** @type {HTMLScriptElement[]} */
const scripts = this.elem.nativeElement.getElementsByTagName(\\"script\\");

for (let i = 0; i < scripts.length; i++) {
Expand Down Expand Up @@ -5468,8 +5423,7 @@ export class FormComponent {
const el = event.currentTarget;
const headers = this.customHeaders || {};
let body: any;
const formData = new FormData(el); // TODO: maybe support null

const formData = new FormData(el);
const formPairs: {
key: string;
value: File | boolean | number | string | FileList;
Expand Down Expand Up @@ -5499,7 +5453,6 @@ export class FormComponent {
value = num;
}
} else if (el.type === \\"file\\") {
// TODO: one vs multiple files
value = el.files;
} else {
value = el.value;
Expand Down Expand Up @@ -5527,14 +5480,11 @@ export class FormComponent {
) {
contentType = \\"multipart/form-data\\";
}
}); // TODO: send as urlEncoded or multipart by default
// because of ease of use and reliability in browser API
// for encoding the form?
});

if (contentType !== \\"application/json\\") {
body = formData;
} else {
// Json
const json = {};
Array.from(formPairs).forEach(({ value, key }) => {
set(json, key, value);
Expand All @@ -5543,10 +5493,7 @@ export class FormComponent {
}

if (contentType && contentType !== \\"multipart/form-data\\") {
if (
/* Zapier doesn't allow content-type header to be sent from browsers */
!(sendWithJs && this.action?.includes(\\"zapier.com\\"))
) {
if (!(sendWithJs && this.action?.includes(\\"zapier.com\\"))) {
headers[\\"content-type\\"] = contentType;
}
}
Expand All @@ -5571,15 +5518,11 @@ export class FormComponent {
}/api/v1/form-submit?apiKey=\${builder.apiKey}&to=\${btoa(
this.sendSubmissionsToEmail || \\"\\"
)}&name=\${encodeURIComponent(this.name || \\"\\")}\`;
fetch(
this.sendSubmissionsTo === \\"email\\" ? formUrl : this.action!,
/* TODO: throw error if no action URL */
{
body,
headers,
method: this.method || \\"post\\",
}
).then(
fetch(this.sendSubmissionsTo === \\"email\\" ? formUrl : this.action!, {
body,
headers,
method: this.method || \\"post\\",
}).then(
async (res) => {
let body;
const contentType = res.headers.get(\\"content-type\\");
Expand All @@ -5591,13 +5534,10 @@ export class FormComponent {
}

if (!res.ok && this.errorMessagePath) {
/* TODO: allow supplying an error formatter function */
let message = get(body, this.errorMessagePath);

if (message) {
if (typeof message !== \\"string\\") {
/* TODO: ideally convert json to yaml so it woul dbe like
error: - email has been taken */
message = JSON.stringify(message);
}

Expand All @@ -5622,13 +5562,11 @@ export class FormComponent {
if (submitSuccessEvent.defaultPrevented) {
return;
}
/* TODO: option to turn this on/off? */

if (this.resetFormOnSubmit !== false) {
this.formRef.nativeElement.reset();
}
}
/* TODO: client side route event first that can be preventDefaulted */

if (this.successUrl) {
if (this.formRef.nativeElement) {
Expand Down Expand Up @@ -5755,7 +5693,6 @@ export class Image {
this.imageLoaded = true;
}
useLazyLoading() {
// TODO: Add more checks here, like testing for real web browsers
return !!this.lazy && this.isBrowser();
}
isBrowser = function isBrowser() {
Expand Down Expand Up @@ -6963,6 +6900,8 @@ export class MyBasicComponentModule {}
"
`;

exports[`Angular with Preserve Imports and File Extensions > jsx > Typescript Test > commentsWithQuotes 1`] = `"JSON5: invalid character '&' at 2:10"`;

exports[`Angular with Preserve Imports and File Extensions > jsx > Typescript Test > componentWithContext 1`] = `
"import { NgModule } from \\"@angular/core\\";
import { CommonModule } from \\"@angular/common\\";
Expand Down Expand Up @@ -8084,7 +8023,6 @@ export class RenderBlock {
const ref = this.context.registeredComponents[componentName];

if (!ref) {
// TODO: Public doc page with more info about this message
console.warn(\`
Could not find a registered this.component named \\"\${componentName}\\".
If you registered it, is the file that registered it imported by the file that needs to render it?\`);
Expand Down Expand Up @@ -8137,11 +8075,6 @@ export class RenderBlock {
componentRef: this.component?.component,
componentOptions: {
...getBlockComponentOptions(this.useBlock),

/**
* These this.attributes are passed to the wrapper element when there is one. If \`noWrap\` is set to true, then
* they are provided to the this.component itself directly.
*/
...(this.shouldWrap
? {}
: {
Expand All @@ -8153,28 +8086,14 @@ export class RenderBlock {
};
}
get useChildren() {
// TO-DO: When should \`canHaveChildren\` dictate rendering?
// This is currently commented out because some Builder components (e.g. Box) do not have \`canHaveChildren: true\`,
// but still receive and need to render children.
// return state.componentInfo?.canHaveChildren ? state.useBlock.children : [];
return this.useBlock.children ?? [];
}
get childrenWithoutParentComponent() {
/**
* When there is no \`componentRef\`, there might still be children that need to be rendered. In this case,
* we render them outside of \`componentRef\`.
* NOTE: We make sure not to render this if \`repeatItemData\` is non-null, because that means we are rendering an array of
* blocks, and the children will be repeated within those blocks.
*/
const shouldRenderChildrenOutsideRef =
!this.component?.component && !this.repeatItemData;
return shouldRenderChildrenOutsideRef ? this.useChildren : [];
}
get repeatItemData() {
/**
* we don't use \`state.useBlock\` here because the processing done within its logic includes evaluating the block's bindings,
* which will not work if there is a repeat.
*/
const { repeat, ...blockWithoutRepeat } = this.block;

if (!repeat?.collection) {
Expand Down Expand Up @@ -8235,7 +8154,6 @@ export class RenderBlock {
if (TARGET === \\"reactNative\\") {
return RenderComponentWithContext;
} else if (TARGET === \\"vue3\\") {
// vue3 expects a string for the this.component tag
return \\"RenderComponent\\";
} else {
return RenderComponent;
Expand Down
Loading
Loading