Skip to content

Commit

Permalink
Merge pull request #7043 from QwikDev/v2-nesting-error-file-location
Browse files Browse the repository at this point in the history 
feat(v2): add nesting error file location
  • Loading branch information
@shairez
shairez authored Nov 4, 2024
2 parents bd98e33 + e24f321 commit 39df9c4
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 26 deletions.
1 change: 1 addition & 0 deletions packages/qwik/src/core/shared/utils/markers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,3 +107,4 @@ export const STREAM_BLOCK_END_COMMENT = 'qkssr-po';
export const Q_PROPS_SEPARATOR = ':';

export const dangerouslySetInnerHTML = 'dangerouslySetInnerHTML';
export const qwikInspectorAttr = 'data-qwik-inspector';
33 changes: 22 additions & 11 deletions packages/qwik/src/core/ssr/ssr-render-jsx.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { isQrl } from '../shared/qrl/qrl-class';
import type { QRL } from '../shared/qrl/qrl.public';
import { Fragment, directGetPropsProxyProp } from '../shared/jsx/jsx-runtime';
import { Slot } from '../shared/jsx/slot.public';
import type { JSXNode, JSXOutput } from '../shared/jsx/types/jsx-node';
import type { DevJSX, JSXNode, JSXOutput } from '../shared/jsx/types/jsx-node';
import type { JSXChildren } from '../shared/jsx/types/jsx-qwik-attributes';
import { SSRComment, SSRRaw, SSRStream, type SSRStreamChildren } from '../shared/jsx/utils.public';
import { trackSignal } from '../use/use-core';
Expand All @@ -17,6 +17,7 @@ import {
QDefaultSlot,
QScopedStyle,
QSlot,
qwikInspectorAttr,
} from '../shared/utils/markers';
import { isPromise } from '../shared/utils/promises';
import { isFunction, type ValueOrPromise } from '../shared/utils/types';
Expand Down Expand Up @@ -179,7 +180,13 @@ function processJSXNode(
// Below, JSXChildren allows functions and regexes, but we assume the dev only uses those as appropriate.
if (typeof type === 'string') {
appendClassIfScopedStyleExists(jsx, options.styleScoped);
appendQwikInspectorAttribute(jsx);
let qwikInspectorAttrValue: string | null = null;
if (isDev && jsx.dev && jsx.type !== 'head') {
qwikInspectorAttrValue = getQwikInspectorAttributeValue(jsx.dev);
if (qInspector) {
appendQwikInspectorAttribute(jsx, qwikInspectorAttrValue);
}
}

const innerHTML = ssr.openElement(
type,
Expand All @@ -195,7 +202,8 @@ function processJSXNode(
jsx.varProps,
ssr.serializationCtx,
options.styleScoped
)
),
qwikInspectorAttrValue
);
if (innerHTML) {
ssr.htmlNode(innerHTML);
Expand Down Expand Up @@ -531,14 +539,17 @@ function getSlotName(host: ISsrNode, jsx: JSXNode, ssr: SSRContainer): string {
return directGetPropsProxyProp(jsx, 'name') || QDefaultSlot;
}

function appendQwikInspectorAttribute(jsx: JSXNode) {
if (isDev && qInspector && jsx.dev && jsx.type !== 'head') {
const sanitizedFileName = jsx.dev.fileName?.replace(/\\/g, '/');
const qwikInspectorAttr = 'data-qwik-inspector';
if (sanitizedFileName && (!jsx.constProps || !(qwikInspectorAttr in jsx.constProps))) {
(jsx.constProps ||= {})[qwikInspectorAttr] =
`${sanitizedFileName}:${jsx.dev.lineNumber}:${jsx.dev.columnNumber}`;
}
function getQwikInspectorAttributeValue(jsxDev: DevJSX): string | null {
const sanitizedFileName = jsxDev.fileName?.replace(/\\/g, '/');
if (sanitizedFileName) {
return `${sanitizedFileName}:${jsxDev.lineNumber}:${jsxDev.columnNumber}`;
}
return null;
}

function appendQwikInspectorAttribute(jsx: JSXNode, qwikInspectorAttrValue: string | null) {
if (qwikInspectorAttrValue && (!jsx.constProps || !(qwikInspectorAttr in jsx.constProps))) {
(jsx.constProps ||= {})[qwikInspectorAttr] = qwikInspectorAttrValue;
}
}

Expand Down
3 changes: 2 additions & 1 deletion packages/qwik/src/core/ssr/ssr-types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ export interface SSRContainer extends Container {
openElement(
elementName: string,
varAttrs: SsrAttrs | null,
constAttrs?: SsrAttrs | null
constAttrs?: SsrAttrs | null,
currentFile?: string | null
): string | undefined;
closeElement(): ValueOrPromise<void>;

Expand Down
20 changes: 12 additions & 8 deletions packages/qwik/src/core/tests/container.spec.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -507,20 +507,22 @@ describe('serializer v2', () => {

describe('element nesting rules', () => {
it('should throw when incorrectly nested elements', async () => {
const filePath = '/some/path/test-file.tsx';
await expect(() =>
withContainer(
(ssr) => {
ssr.openElement('body', []);
ssr.openElement('p', []);
ssr.openElement('body', [], null, filePath);
ssr.openElement('p', [], null, filePath);
ssr.openFragment([]);
ssr.openElement('b', []);
ssr.openElement('div', []);
ssr.openElement('b', [], null, filePath);
ssr.openElement('div', [], null, filePath);
},
{ containerTag: 'html' }
)
).rejects.toThrowError(
[
`SsrError(tag): HTML rules do not allow '<div>' at this location.`,
`SsrError(tag): Error found in file: ${filePath}`,
`HTML rules do not allow '<div>' at this location.`,
` (The HTML parser will try to recover by auto-closing or inserting additional tags which will confuse Qwik when it resumes.)`,
` Offending tag: <div>`,
` Existing tag context:`,
Expand All @@ -533,15 +535,17 @@ describe('serializer v2', () => {
);
});
it('should throw when adding content to empty elements', async () => {
const filePath = '/some/path/test-file.tsx';
await expect(() =>
withContainer((ssr) => {
ssr.openElement('img', []);
ssr.openElement('img', [], null, filePath);
ssr.openFragment([]);
ssr.openElement('div', []);
ssr.openElement('div', [], null, filePath);
})
).rejects.toThrowError(
[
`SsrError(tag): HTML rules do not allow '<div>' at this location.`,
`SsrError(tag): Error found in file: ${filePath}`,
`HTML rules do not allow '<div>' at this location.`,
` (The HTML parser will try to recover by auto-closing or inserting additional tags which will confuse Qwik when it resumes.)`,
` Offending tag: <div>`,
` Existing tag context:`,
Expand Down
23 changes: 17 additions & 6 deletions packages/qwik/src/server/ssr-container.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,8 @@ class SSRContainer extends _SharedContainer implements ISSRContainer {
openElement(
elementName: string,
varAttrs: SsrAttrs | null,
constAttrs?: SsrAttrs | null
constAttrs?: SsrAttrs | null,
currentFile?: string | null
): string | undefined {
let innerHTML: string | undefined = undefined;
this.lastNode = null;
Expand All @@ -352,7 +353,7 @@ class SSRContainer extends _SharedContainer implements ISSRContainer {
vNodeData_incrementElementCount(this.currentElementFrame.vNodeData);
}

this.createAndPushFrame(elementName, this.depthFirstElementCount++);
this.createAndPushFrame(elementName, this.depthFirstElementCount++, currentFile);
this.write('<');
this.write(elementName);
if (varAttrs) {
Expand Down Expand Up @@ -987,7 +988,11 @@ class SSRContainer extends _SharedContainer implements ISSRContainer {
return elementIdx;
}

private createAndPushFrame(elementName: string, depthFirstElementIdx: number) {
private createAndPushFrame(
elementName: string,
depthFirstElementIdx: number,
currentFile?: string | null
) {
let tagNesting: TagNesting = TagNesting.ANYTHING;
if (isDev) {
if (!this.currentElementFrame) {
Expand All @@ -1002,12 +1007,18 @@ class SSRContainer extends _SharedContainer implements ISSRContainer {
frames.unshift(frame);
frame = frame.parent;
}
const text: string[] = [
const text: string[] = [];

if (currentFile) {
text.push(`Error found in file: ${currentFile}`);
}

text.push(
`HTML rules do not allow '<${elementName}>' at this location.`,
` (The HTML parser will try to recover by auto-closing or inserting additional tags which will confuse Qwik when it resumes.)`,
` Offending tag: <${elementName}>`,
` Existing tag context:`,
];
` Existing tag context:`
);
let indent = ' ';
let lastName = '';
for (const frame of frames) {
Expand Down

0 comments on commit 39df9c4

Please sign in to comment.