Skip to content

Commit 93ffb4d

Browse files
authored
chore: squelch warnings, robustify (#13034)
This also flagged a few places where we were doing things wrongly (createRawSnippet validation of render function was buggy, we had an unnecessary console.trace which duplicated the existing trace in the warning, and the parent/child SSR element state was getting corrupted when unrelated errors happened). The fact that the test output now looks a lot cleaner is just a nice bonus
1 parent 8cda791 commit 93ffb4d

File tree

11 files changed

+89
-56
lines changed

11 files changed

+89
-56
lines changed

packages/svelte/src/internal/client/dom/blocks/snippet.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ export function createRawSnippet(fn) {
9292
var fragment = create_fragment_from_html(html);
9393
element = /** @type {Element} */ (get_first_child(fragment));
9494

95-
if (DEV && (get_next_sibling(element) !== null || element.nodeType !== 3)) {
95+
if (DEV && (get_next_sibling(element) !== null || element.nodeType !== 1)) {
9696
w.invalid_raw_snippet_render();
9797
}
9898

packages/svelte/src/internal/client/render.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -290,10 +290,10 @@ let mounted_components = new WeakMap();
290290
*/
291291
export function unmount(component) {
292292
const fn = mounted_components.get(component);
293-
if (DEV && !fn) {
293+
294+
if (fn) {
295+
fn();
296+
} else if (DEV) {
294297
w.lifecycle_double_unmount();
295-
// eslint-disable-next-line no-console
296-
console.trace('stack trace');
297298
}
298-
fn?.();
299299
}

packages/svelte/src/internal/server/dev.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,10 @@ function print_error(payload, parent, child) {
5050
payload.head.out += `<script>console.error(${JSON.stringify(message)})</script>`;
5151
}
5252

53+
export function reset_elements() {
54+
parent = null;
55+
}
56+
5357
/**
5458
* @param {Payload} payload
5559
* @param {string} tag

packages/svelte/src/internal/server/index.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { current_component, pop, push } from './context.js';
1616
import { EMPTY_COMMENT, BLOCK_CLOSE, BLOCK_OPEN } from './hydration.js';
1717
import { validate_store } from '../shared/validate.js';
1818
import { is_boolean_attribute, is_void } from '../../utils.js';
19+
import { reset_elements } from './dev.js';
1920

2021
// https://html.spec.whatwg.org/multipage/syntax.html#attributes-2
2122
// https://infra.spec.whatwg.org/#noncharacter
@@ -100,6 +101,11 @@ export function render(component, options = {}) {
100101
on_destroy = [];
101102
payload.out += BLOCK_OPEN;
102103

104+
if (DEV) {
105+
// prevent parent/child element state being corrupted by a bad render
106+
reset_elements();
107+
}
108+
103109
if (options.context) {
104110
push();
105111
/** @type {Component} */ (current_component).c = options.context;

packages/svelte/tests/runtime-legacy/samples/store-dev-mode-error/main.svelte

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22
export let count;
33
</script>
44

5-
<button on:click="{() => count.update(n => n + 1)}">count {$count}</button>
5+
<button on:click={() => count.update((n) => n + 1)}>count {$count}</button>

packages/svelte/tests/runtime-legacy/samples/transition-js-destroyed-before-end/_config.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,7 @@ export default test({
1515
component.$destroy();
1616

1717
raf.tick(100);
18-
}
18+
},
19+
20+
warnings: ['Tried to unmount a component that was not mounted']
1921
});

packages/svelte/tests/runtime-legacy/shared.ts

Lines changed: 33 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ export interface RuntimeTest<Props extends Record<string, any> = Record<string,
6060
};
6161
logs: any[];
6262
warnings: any[];
63+
errors: any[];
6364
hydrate: Function;
6465
}) => void | Promise<void>;
6566
test_ssr?: (args: { logs: any[]; assert: Assert }) => void | Promise<void>;
@@ -70,6 +71,7 @@ export interface RuntimeTest<Props extends Record<string, any> = Record<string,
7071
error?: string;
7172
runtime_error?: string;
7273
warnings?: string[];
74+
errors?: string[];
7375
expect_unhandled_rejections?: boolean;
7476
withoutNormalizeHtml?: boolean | 'only-strip-comments';
7577
recover?: boolean;
@@ -96,6 +98,15 @@ afterAll(() => {
9698
process.removeListener('unhandledRejection', unhandled_rejection_handler);
9799
});
98100

101+
// eslint-disable-next-line no-console
102+
let console_log = console.log;
103+
104+
// eslint-disable-next-line no-console
105+
let console_warn = console.warn;
106+
107+
// eslint-disable-next-line no-console
108+
let console_error = console.error;
109+
99110
export function runtime_suite(runes: boolean) {
100111
return suite_with_variants<RuntimeTest, 'hydrate' | 'ssr' | 'dom', CompileOptions>(
101112
['dom', 'hydrate', 'ssr'],
@@ -171,11 +182,9 @@ async function run_test_variant(
171182
) {
172183
let unintended_error = false;
173184

174-
// eslint-disable-next-line no-console
175-
const { log, warn } = console;
176-
177185
let logs: string[] = [];
178186
let warnings: string[] = [];
187+
let errors: string[] = [];
179188
let manual_hydrate = false;
180189

181190
{
@@ -214,6 +223,13 @@ async function run_test_variant(
214223
}
215224
};
216225
}
226+
227+
if (str.slice(0, i).includes('errors') || config.errors) {
228+
// eslint-disable-next-line no-console
229+
console.error = (...args) => {
230+
errors.push(...args);
231+
};
232+
}
217233
}
218234

219235
try {
@@ -311,15 +327,6 @@ async function run_test_variant(
311327

312328
config.before_test?.();
313329

314-
// eslint-disable-next-line no-console
315-
const error = console.error;
316-
// eslint-disable-next-line no-console
317-
console.error = (error) => {
318-
if (typeof error === 'string' && error.startsWith('Hydration failed')) {
319-
throw new Error(error);
320-
}
321-
};
322-
323330
let instance: any;
324331
let props: any;
325332
let hydrate_fn: Function = () => {
@@ -357,9 +364,6 @@ async function run_test_variant(
357364
});
358365
}
359366

360-
// eslint-disable-next-line no-console
361-
console.error = error;
362-
363367
if (config.error) {
364368
unintended_error = true;
365369
assert.fail('Expected a runtime error');
@@ -395,6 +399,7 @@ async function run_test_variant(
395399
compileOptions,
396400
logs,
397401
warnings,
402+
errors,
398403
hydrate: hydrate_fn
399404
});
400405
}
@@ -412,12 +417,20 @@ async function run_test_variant(
412417

413418
if (config.warnings) {
414419
assert.deepEqual(warnings, config.warnings);
415-
} else if (warnings.length && console.warn === warn) {
420+
} else if (warnings.length && console.warn === console_warn) {
416421
unintended_error = true;
417-
warn.apply(console, warnings);
422+
console_warn.apply(console, warnings);
418423
assert.fail('Received unexpected warnings');
419424
}
420425

426+
if (config.errors) {
427+
assert.deepEqual(errors, config.errors);
428+
} else if (errors.length && console.error === console_error) {
429+
unintended_error = true;
430+
console_error.apply(console, errors);
431+
assert.fail('Received unexpected errors');
432+
}
433+
421434
assert_html_equal(
422435
target.innerHTML,
423436
'',
@@ -440,8 +453,9 @@ async function run_test_variant(
440453
throw err;
441454
}
442455
} finally {
443-
console.log = log;
444-
console.warn = warn;
456+
console.log = console_log;
457+
console.warn = console_warn;
458+
console.error = console_error;
445459

446460
config.after_test?.();
447461

packages/svelte/tests/runtime-runes/samples/event-global-hydration-error-cleanup/_config.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,18 @@ import { test } from '../../test';
33

44
export default test({
55
html: '<p><p>invalid</p></p>',
6+
67
mode: ['hydrate'],
8+
79
recover: true,
10+
811
test({ assert, target, logs }) {
912
target.click();
1013
flushSync();
1114
assert.deepEqual(logs, ['body', 'document', 'window']);
12-
}
15+
},
16+
17+
warnings: [
18+
'Hydration failed because the initial UI does not match what was rendered on the server'
19+
]
1320
});
Lines changed: 8 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,5 @@
11
import { test } from '../../test';
22

3-
let console_error = console.error;
4-
5-
/**
6-
* @type {any[]}
7-
*/
8-
const log = [];
9-
103
export default test({
114
compileOptions: {
125
dev: true
@@ -16,27 +9,14 @@ export default test({
169

1710
recover: true,
1811

19-
before_test() {
20-
console.error = (x) => {
21-
log.push(x);
22-
};
23-
},
12+
mode: ['hydrate'],
2413

25-
after_test() {
26-
console.error = console_error;
27-
log.length = 0;
28-
},
14+
errors: [
15+
'node_invalid_placement_ssr: `<p>` (main.svelte:6:0) cannot contain `<h1>` (h1.svelte:1:0)\n\nThis can cause content to shift around as the browser repairs the HTML, and will likely result in a `hydration_mismatch` warning.',
16+
'node_invalid_placement_ssr: `<form>` (main.svelte:9:0) cannot contain `<form>` (form.svelte:1:0)\n\nThis can cause content to shift around as the browser repairs the HTML, and will likely result in a `hydration_mismatch` warning.'
17+
],
2918

30-
async test({ assert, variant }) {
31-
if (variant === 'hydrate') {
32-
assert.equal(
33-
log[0].split('\n')[0],
34-
'node_invalid_placement_ssr: `<p>` (main.svelte:6:0) cannot contain `<h1>` (h1.svelte:1:0)'
35-
);
36-
assert.equal(
37-
log[1].split('\n')[0],
38-
'node_invalid_placement_ssr: `<form>` (main.svelte:9:0) cannot contain `<form>` (form.svelte:1:0)'
39-
);
40-
}
41-
}
19+
warnings: [
20+
'Hydration failed because the initial UI does not match what was rendered on the server'
21+
]
4222
});

packages/svelte/tests/server-side-rendering/samples/invalid-nested-svelte-element/_config.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,9 @@ import { test } from '../../test';
33
export default test({
44
compileOptions: {
55
dev: true
6-
}
6+
},
7+
8+
errors: [
9+
'node_invalid_placement_ssr: `<p>` (packages/svelte/tests/server-side-rendering/samples/invalid-nested-svelte-element/main.svelte:1:0) cannot contain `<p>` (packages/svelte/tests/server-side-rendering/samples/invalid-nested-svelte-element/main.svelte:2:1)\n\nThis can cause content to shift around as the browser repairs the HTML, and will likely result in a `hydration_mismatch` warning.'
10+
]
711
});

packages/svelte/tests/server-side-rendering/test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,21 @@ interface SSRTest extends BaseTest {
1616
compileOptions?: Partial<CompileOptions>;
1717
props?: Record<string, any>;
1818
withoutNormalizeHtml?: boolean;
19+
errors?: string[];
1920
}
2021

22+
// eslint-disable-next-line no-console
23+
let console_error = console.error;
24+
2125
const { test, run } = suite<SSRTest>(async (config, test_dir) => {
2226
await compile_directory(test_dir, 'server', config.compileOptions);
2327

28+
const errors: string[] = [];
29+
30+
console.error = (...args) => {
31+
errors.push(...args);
32+
};
33+
2434
const Component = (await import(`${test_dir}/_output/server/main.svelte.js`)).default;
2535
const expected_html = try_read_file(`${test_dir}/_expected.html`);
2636
const rendered = render(Component, { props: config.props || {} });
@@ -64,6 +74,12 @@ const { test, run } = suite<SSRTest>(async (config, test_dir) => {
6474
}
6575
}
6676
}
77+
78+
if (errors.length > 0) {
79+
assert.deepEqual(config.errors, errors);
80+
}
81+
82+
console.error = console_error;
6783
});
6884

6985
export { test };

0 commit comments

Comments
 (0)