Skip to content

Commit

Permalink
Merge pull request #7196 from QwikDev/v2-textarea-null-value
Browse files Browse the repository at this point in the history
fix: textarea with null value
  • Loading branch information
Varixo authored Dec 27, 2024
2 parents cef6a0d + ef92d10 commit c029c32
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 26 deletions.
5 changes: 5 additions & 0 deletions .changeset/red-trains-deny.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@qwik.dev/core': patch
---

fix: textarea with null value
27 changes: 21 additions & 6 deletions packages/qwik/src/core/client/vnode-diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ import {
} from '../signal/signal-subscriber';
import { serializeAttribute } from '../shared/utils/styles';
import { QError, qError } from '../shared/error/error';
import { getFileLocationFromJsx } from '../shared/utils/jsx-filename';

export type ComponentQueue = Array<VNode>;

Expand Down Expand Up @@ -594,7 +595,11 @@ export const vnode_diff = (
*
* @returns {boolean}
*/
function createNewElement(jsx: JSXNodeInternal, elementName: string): boolean {
function createNewElement(
jsx: JSXNodeInternal,
elementName: string,
currentFile?: string | null
): boolean {
const element = createElementWithNamespace(elementName);

const { constProps } = jsx;
Expand Down Expand Up @@ -629,6 +634,8 @@ export const vnode_diff = (
} else if (typeof value === 'function') {
value(element);
continue;
} else {
throw qError(QError.invalidRefValue, [currentFile]);
}
}

Expand All @@ -653,13 +660,13 @@ export const vnode_diff = (
}

if (elementName === 'textarea' && key === 'value') {
if (typeof value !== 'string') {
if (value && typeof value !== 'string') {
if (isDev) {
throw qError(QError.wrongTextareaValue);
throw qError(QError.wrongTextareaValue, [currentFile, value]);
}
continue;
}
(element as HTMLTextAreaElement).value = escapeHTML(value as string);
(element as HTMLTextAreaElement).value = escapeHTML((value as string) || '');
continue;
}

Expand Down Expand Up @@ -705,6 +712,7 @@ export const vnode_diff = (
vCurrent && vnode_isElementVNode(vCurrent) && elementName === vnode_getElementName(vCurrent);
const jsxKey: string | null = jsx.key;
let needsQDispatchEventPatch = false;
const currentFile = getFileLocationFromJsx(jsx.dev);
if (!isSameElementName || jsxKey !== getKey(vCurrent)) {
// So we have a key and it does not match the current node.
// We need to do a forward search to find it.
Expand Down Expand Up @@ -732,7 +740,8 @@ export const vnode_diff = (
mapArray_set(jsxAttrs, ELEMENT_KEY, jsxKey, 0);
}
const vNode = (vNewNode || vCurrent) as ElementVNode;
needsQDispatchEventPatch = setBulkProps(vNode, jsxAttrs) || needsQDispatchEventPatch;
needsQDispatchEventPatch =
setBulkProps(vNode, jsxAttrs, currentFile) || needsQDispatchEventPatch;
if (needsQDispatchEventPatch) {
// Event handler needs to be patched onto the element.
const element = vnode_getNode(vNode) as QElement;
Expand All @@ -758,7 +767,11 @@ export const vnode_diff = (
}

/** @param tag Returns true if `qDispatchEvent` needs patching */
function setBulkProps(vnode: ElementVNode, srcAttrs: ClientAttrs): boolean {
function setBulkProps(
vnode: ElementVNode,
srcAttrs: ClientAttrs,
currentFile?: string | null
): boolean {
vnode_ensureElementInflated(vnode);
const dstAttrs = vnode as ClientAttrs;
let srcIdx = 0;
Expand All @@ -783,6 +796,8 @@ export const vnode_diff = (
} else if (typeof value === 'function') {
value(element);
return;
} else {
throw qError(QError.invalidRefValue, [currentFile]);
}
}

Expand Down
4 changes: 2 additions & 2 deletions packages/qwik/src/core/shared/error/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,15 @@ export const codeToText = (code: number, ...parts: any[]): string => {
'SsrError(tag): {{0}}', // 29
'QRLs can not be resolved because it does not have an attached container. This means that the QRL does not know where it belongs inside the DOM, so it cant dynamically import() from a relative path.', // 30
'QRLs can not be dynamically resolved, because it does not have a chunk path', // 31
'The JSX ref attribute must be a Signal', // 32
'{{0}}\nThe JSX ref attribute must be a Signal', // 32
'Serialization Error: Deserialization of data type {{0}} is not implemented', // 33
'Serialization Error: Expected vnode for ref prop, but got {{0}}', // 34
'Serialization Error: Cannot allocate data type {{0}}', // 35
'Serialization Error: Missing root id for {{0}}', // 36
'Serialization Error: Serialization of data type {{0}} is not implemented', // 37
'Serialization Error: Unvisited {{0}}', // 38
'Serialization Error: Missing QRL chunk for {{0}}', // 39
'The value of the textarea must be a string', // 40
'{{0}}\nThe value of the textarea must be a string found {{1}}', // 40
'Unable to find q:container', // 41
"Element must have 'q:container' attribute.", // 42
'Unknown vnode type {{0}}.', // 43
Expand Down
12 changes: 12 additions & 0 deletions packages/qwik/src/core/shared/utils/jsx-filename.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import type { DevJSX } from '../jsx/types/jsx-node';

export function getFileLocationFromJsx(jsxDev?: DevJSX): string | null {
if (!jsxDev) {
return null;
}
const sanitizedFileName = jsxDev.fileName?.replace(/\\/g, '/');
if (sanitizedFileName) {
return `${sanitizedFileName}:${jsxDev.lineNumber}:${jsxDev.columnNumber}`;
}
return null;
}
13 changes: 3 additions & 10 deletions packages/qwik/src/core/ssr/ssr-render-jsx.ts
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 { DevJSX, JSXNodeInternal, JSXOutput } from '../shared/jsx/types/jsx-node';
import type { JSXNodeInternal, 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 { trackSignalAndAssignHost } from '../use/use-core';
Expand Down Expand Up @@ -35,6 +35,7 @@ import type { ISsrComponentFrame, ISsrNode, SSRContainer, SsrAttrs } from './ssr
import { qInspector } from '../shared/utils/qdev';
import { serializeAttribute } from '../shared/utils/styles';
import { QError, qError } from '../shared/error/error';
import { getFileLocationFromJsx } from '../shared/utils/jsx-filename';

class ParentComponentData {
constructor(
Expand Down Expand Up @@ -182,7 +183,7 @@ function processJSXNode(
appendClassIfScopedStyleExists(jsx, options.styleScoped);
let qwikInspectorAttrValue: string | null = null;
if (isDev && jsx.dev && jsx.type !== 'head') {
qwikInspectorAttrValue = getQwikInspectorAttributeValue(jsx.dev);
qwikInspectorAttrValue = getFileLocationFromJsx(jsx.dev);
if (qInspector) {
appendQwikInspectorAttribute(jsx, qwikInspectorAttrValue);
}
Expand Down Expand Up @@ -540,14 +541,6 @@ function getSlotName(host: ISsrNode, jsx: JSXNodeInternal, ssr: SSRContainer): s
return directGetPropsProxyProp(jsx, 'name') || QDefaultSlot;
}

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: JSXNodeInternal, qwikInspectorAttrValue: string | null) {
if (qwikInspectorAttrValue && (!jsx.constProps || !(qwikInspectorAttr in jsx.constProps))) {
(jsx.constProps ||= {})[qwikInspectorAttr] = qwikInspectorAttrValue;
Expand Down
22 changes: 21 additions & 1 deletion packages/qwik/src/core/tests/component.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
useTask$,
useVisibleTask$,
type JSXOutput,
type PropsOf,
type Signal as SignalType,
} from '@qwik.dev/core';
import { domRender, ssrRenderToDom, trigger } from '@qwik.dev/core/testing';
Expand Down Expand Up @@ -480,6 +481,25 @@ describe.each([
await expect(document.querySelector('textarea')).toMatchDOM(<textarea>value 123!</textarea>);
});

it('should render textarea without error', async () => {
const Textarea = component$<PropsOf<'textarea'>>(
({ ['bind:value']: valueSig, value, ...props }) => {
return (
<>
<textarea {...props} value={valueSig ? valueSig.value : value} />
</>
);
}
);

const Cmp = component$(() => {
return <Textarea />;
});

const { document } = await render(<Cmp />, { debug });
await expect(document.querySelector('textarea')).toMatchDOM(<textarea></textarea>);
});

it('should not render textarea value for non-text value', async () => {
const qErrorSpy = vi.spyOn(qError, 'qError');
const Cmp = component$(() => {
Expand All @@ -502,7 +522,7 @@ describe.each([
);
} catch (e) {
expect((e as Error).message).toBeDefined();
expect(qErrorSpy).toHaveBeenCalledWith(QError.wrongTextareaValue);
expect(qErrorSpy).toHaveBeenCalledWith(QError.wrongTextareaValue, expect.anything());
}
});

Expand Down
19 changes: 12 additions & 7 deletions packages/qwik/src/server/ssr-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -363,13 +363,13 @@ class SSRContainer extends _SharedContainer implements ISSRContainer {
this.write('<');
this.write(elementName);
if (varAttrs) {
innerHTML = this.writeAttrs(elementName, varAttrs, false);
innerHTML = this.writeAttrs(elementName, varAttrs, false, currentFile);
}
this.write(' ' + Q_PROPS_SEPARATOR);
// Domino sometimes does not like empty attributes, so we need to add a empty value
isDev && this.write('=""');
if (constAttrs && constAttrs.length) {
innerHTML = this.writeAttrs(elementName, constAttrs, true) || innerHTML;
innerHTML = this.writeAttrs(elementName, constAttrs, true, currentFile) || innerHTML;
}
this.write('>');
this.lastNode = null;
Expand Down Expand Up @@ -1115,7 +1115,12 @@ class SSRContainer extends _SharedContainer implements ISSRContainer {
}
}

private writeAttrs(tag: string, attrs: SsrAttrs, isConst: boolean): string | undefined {
private writeAttrs(
tag: string,
attrs: SsrAttrs,
isConst: boolean,
currentFile?: string | null
): string | undefined {
let innerHTML: string | undefined = undefined;
if (attrs.length) {
for (let i = 0; i < attrs.length; i++) {
Expand Down Expand Up @@ -1146,7 +1151,7 @@ class SSRContainer extends _SharedContainer implements ISSRContainer {
value(new DomRef(lastNode));
continue;
} else {
throw qError(QError.invalidRefValue);
throw qError(QError.invalidRefValue, [currentFile]);
}
}

Expand All @@ -1171,13 +1176,13 @@ class SSRContainer extends _SharedContainer implements ISSRContainer {
}

if (tag === 'textarea' && key === 'value') {
if (typeof value !== 'string') {
if (value && typeof value !== 'string') {
if (isDev) {
throw qError(QError.wrongTextareaValue);
throw qError(QError.wrongTextareaValue, [currentFile, value]);
}
continue;
}
innerHTML = escapeHTML(value);
innerHTML = escapeHTML(value || '');
key = QContainerAttr;
value = QContainerValue.TEXT;
}
Expand Down

0 comments on commit c029c32

Please sign in to comment.