Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 8 additions & 1 deletion src/component/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ export const defineComponent = <TProps extends Record<string, unknown>>(
private props = {} as TProps;
/** Tracks missing required props for validation during connectedCallback */
private missingRequiredProps = new Set<string>();
/** Tracks whether the component has completed its initial mount */
private hasMounted = false;

constructor() {
super();
Expand Down Expand Up @@ -56,6 +58,7 @@ export const defineComponent = <TProps extends Record<string, unknown>>(
definition.beforeMount?.call(this);
definition.connected?.call(this);
this.render();
this.hasMounted = true;
} catch (error) {
this.handleError(error as Error);
}
Expand All @@ -82,7 +85,11 @@ export const defineComponent = <TProps extends Record<string, unknown>>(
): void {
try {
this.syncProps();
this.render(true);
// Only re-render if the component has completed its initial mount
// This prevents pre-mount renders when attributes are set during element upgrade
if (this.hasMounted) {
this.render(true);
}
Comment thread
JosunLP marked this conversation as resolved.
} catch (error) {
this.handleError(error as Error);
}
Expand Down
2 changes: 1 addition & 1 deletion src/core/utils/function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ export function once<TArgs extends unknown[], TResult>(
let result: TResult;
return (...args: TArgs) => {
if (!hasRun) {
hasRun = true;
result = fn(...args);
hasRun = true;
Comment thread
JosunLP marked this conversation as resolved.
}
return result;
};
Expand Down
4 changes: 2 additions & 2 deletions src/view/directives/class.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ export const handleClass: DirectiveHandler = (el, expression, context, cleanups)
// Track class regardless of condition - toggle handles add/remove
newClasses.add(className);
}
} else if (expression.includes('[')) {
// Array syntax: [class1, class2]
} else if (/^\s*\[/.test(expression)) {
// Array literal syntax: [class1, class2]
Comment thread
JosunLP marked this conversation as resolved.
const classes = evaluate<string[]>(expression, context);
if (Array.isArray(classes)) {
for (const cls of classes) {
Expand Down
9 changes: 9 additions & 0 deletions src/view/mount.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,15 @@ export const mount = (
throw new Error(`bQuery view: Element "${selector}" not found.`);
}

// Reject if root element has bq-for directive
// bq-for replaces the element with a placeholder comment, which would leave View.el detached
if (el.hasAttribute(`${prefix}-for`)) {
throw new Error(
`bQuery view: Cannot mount on element with ${prefix}-for directive. ` +
`Wrap the ${prefix}-for element in a container instead.`
);
}
Comment thread
JosunLP marked this conversation as resolved.

const cleanups: CleanupFn[] = [];

const handlers: DirectiveHandlers = {
Expand Down
100 changes: 100 additions & 0 deletions tests/component.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,106 @@ describe('component/defineComponent', () => {
el.remove();
});

it('does not render when attributes are set before connectedCallback', () => {
const tagName = `test-define-pre-mount-${Date.now()}`;
let renderCount = 0;

const ElementClass = defineComponent<{ value: string }>(tagName, {
props: {
value: { type: String, default: 'default' },
},
render: ({ props }) => {
renderCount++;
return html`<div>Value: ${props.value}</div>`;
},
});

customElements.define(tagName, ElementClass);

// Create element and set attributes BEFORE connecting to DOM
const el = document.createElement(tagName);
el.setAttribute('value', 'initial');
el.setAttribute('value', 'changed');
el.setAttribute('value', 'final');

// Should not have rendered yet (attributeChangedCallback called but hasMounted is false)
expect(renderCount).toBe(0);

// Connect to DOM - this triggers connectedCallback and first render
document.body.appendChild(el);

// Should render exactly once with the final attribute value
expect(renderCount).toBe(1);
expect(el.shadowRoot?.innerHTML).toContain('Value: final');

el.remove();
});

it('renders only once when element is upgraded with attributes already set', () => {
const tagName = `test-define-upgrade-${Date.now()}`;
let renderCount = 0;

const ElementClass = defineComponent<{ count: number }>(tagName, {
props: {
count: { type: Number, default: 0 },
},
render: ({ props }) => {
renderCount++;
return html`<div>Count: ${props.count}</div>`;
},
});

// Create element with innerHTML (element exists but not yet upgraded)
const container = document.createElement('div');
container.innerHTML = `<${tagName} count="99"></${tagName}>`;
const el = container.querySelector(tagName)!;

// Define the component - this triggers upgrade
customElements.define(tagName, ElementClass);

// Append to body to trigger connectedCallback
document.body.appendChild(el);

// Should render exactly once despite attribute being set during construction
expect(renderCount).toBe(1);
expect(el.shadowRoot?.innerHTML).toContain('Count: 99');

el.remove();
});

it('attributeChangedCallback triggers re-render after mount', () => {
const tagName = `test-define-post-mount-rerender-${Date.now()}`;
let renderCount = 0;

const ElementClass = defineComponent<{ value: string }>(tagName, {
props: {
value: { type: String, default: 'initial' },
},
render: ({ props }) => {
renderCount++;
return html`<div>Value: ${props.value}</div>`;
},
});

customElements.define(tagName, ElementClass);
const el = document.createElement(tagName);
document.body.appendChild(el);

expect(renderCount).toBe(1);
expect(el.shadowRoot?.innerHTML).toContain('Value: initial');

// Now that component is mounted, attribute changes should trigger re-renders
el.setAttribute('value', 'updated1');
expect(renderCount).toBe(2);
expect(el.shadowRoot?.innerHTML).toContain('Value: updated1');

el.setAttribute('value', 'updated2');
expect(renderCount).toBe(3);
expect(el.shadowRoot?.innerHTML).toContain('Value: updated2');

el.remove();
});

it('instances sanitize rendered markup for security', () => {
const tagName = `test-define-sanitize-${Date.now()}`;
const ElementClass = defineComponent(tagName, {
Expand Down
79 changes: 79 additions & 0 deletions tests/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,85 @@ describe('utils/string helpers', () => {
});
});

describe('utils/once', () => {
it('executes function only once', () => {
let callCount = 0;
const fn = utils.once(() => {
callCount++;
return 'result';
});

const result1 = fn();
const result2 = fn();
const result3 = fn();

expect(callCount).toBe(1);
expect(result1).toBe('result');
expect(result2).toBe('result');
expect(result3).toBe('result');
});

it('returns cached result on subsequent calls', () => {
const fn = utils.once(() => ({ value: Math.random() }));

const result1 = fn();
const result2 = fn();

expect(result2).toBe(result1);
});

it('does not cache failures when function throws', () => {
let callCount = 0;
const fn = utils.once(() => {
callCount++;
if (callCount === 1) {
throw new Error('First call fails');
}
return 'success';
});

// First call should throw
expect(() => fn()).toThrow('First call fails');
expect(callCount).toBe(1);

// Second call should retry and succeed
const result = fn();
expect(callCount).toBe(2);
expect(result).toBe('success');

// Third call should return cached success result
const result2 = fn();
expect(callCount).toBe(2); // No additional call
expect(result2).toBe('success');
});

it('retries on each call until function succeeds', () => {
let callCount = 0;
const fn = utils.once(() => {
callCount++;
if (callCount < 3) {
throw new Error('Not ready yet');
}
return 'finally ready';
});

expect(() => fn()).toThrow('Not ready yet');
expect(callCount).toBe(1);

expect(() => fn()).toThrow('Not ready yet');
expect(callCount).toBe(2);

const result = fn();
expect(callCount).toBe(3);
expect(result).toBe('finally ready');

// Should not call again after success
const result2 = fn();
expect(callCount).toBe(3);
expect(result2).toBe('finally ready');
});
});

describe('utils/sleep', () => {
it('returns a promise', () => {
const result = utils.sleep(0);
Expand Down
64 changes: 64 additions & 0 deletions tests/view.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,29 @@ describe('View', () => {
it('should throw for non-existent selector', () => {
expect(() => mount('#nonexistent', {})).toThrow('not found');
});

it('should reject mounting on element with bq-for directive', () => {
container.innerHTML = '<div bq-for="item in items" bq-text="item"></div>';
const items = signal([1, 2, 3]);
const div = container.querySelector('div')!;

expect(() => mount(div, { items })).toThrow(
'Cannot mount on element with bq-for directive'
);
expect(() => mount(div, { items })).toThrow('Wrap the bq-for element in a container');
});

it('should mount successfully when bq-for is on child element', () => {
container.innerHTML = '<div><ul><li bq-for="item in items" bq-text="item"></li></ul></div>';
const items = signal([1, 2, 3]);

// Should not throw when mounting on container
view = mount(container, { items });

expect(view.el).toBe(container);
const listItems = container.querySelectorAll('li');
expect(listItems.length).toBe(3);
});
});

describe('bq-text', () => {
Expand Down Expand Up @@ -169,6 +192,47 @@ describe('View', () => {
const div = container.querySelector('div')!;
expect(div.classList.contains('primary')).toBe(true);
});

it('should handle bracket property access correctly', () => {
container.innerHTML = '<div bq-class="item[\'className\']"></div>';
const item = signal({ className: 'dynamic-class' });

view = mount(container, { item });

const div = container.querySelector('div')!;
expect(div.classList.contains('dynamic-class')).toBe(true);

// Update property value
item.value = { className: 'new-class' };
expect(div.classList.contains('new-class')).toBe(true);
expect(div.classList.contains('dynamic-class')).toBe(false);
});

it('should handle array literal syntax correctly', () => {
container.innerHTML = '<div bq-class="[\'foo\', \'bar\']"></div>';

view = mount(container, {});

const div = container.querySelector('div')!;
expect(div.classList.contains('foo')).toBe(true);
expect(div.classList.contains('bar')).toBe(true);
});

it('should distinguish bracket access from array literals', () => {
// Test that obj['key'] is NOT treated as array literal
container.innerHTML = '<div id="test1" bq-class="config[\'activeClass\']"></div>';
const config = signal({ activeClass: 'enabled' });

view = mount(container, { config });

const div = container.querySelector('#test1')!;
expect(div.classList.contains('enabled')).toBe(true);

// Verify it's reactive
config.value = { activeClass: 'disabled' };
expect(div.classList.contains('disabled')).toBe(true);
expect(div.classList.contains('enabled')).toBe(false);
});
});

describe('bq-style', () => {
Expand Down
Loading