From 843573a02818a63f609986c86e10d7adfd662690 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Thu, 30 May 2024 09:23:28 -0700 Subject: [PATCH 1/9] chore: don't swallow rejected promise errors --- eslint.config.js | 10 +++++++++- .../client/dom/elements/custom-element.js | 19 ++++++++++++------- .../src/internal/client/dom/elements/misc.js | 19 ++++++++++++------- .../svelte/src/internal/client/runtime.js | 7 ++++++- packages/svelte/src/motion/spring.js | 11 ++++++++--- playgrounds/demo/.prettierignore | 1 - playgrounds/demo/package.json | 4 +--- playgrounds/sandbox/run.js | 4 +++- playgrounds/sandbox/tsconfig.json | 2 +- 9 files changed, 52 insertions(+), 25 deletions(-) delete mode 100644 playgrounds/demo/.prettierignore diff --git a/eslint.config.js b/eslint.config.js index c15c17e7dca9..528fe94b7590 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -5,10 +5,19 @@ import lube from 'eslint-plugin-lube'; export default [ ...svelte_config, { + languageOptions: { + parserOptions: { + project: true + } + }, plugins: { lube }, rules: { + '@typescript-eslint/await-thenable': 'error', + '@typescript-eslint/no-floating-promises': 'error', + '@typescript-eslint/no-misused-promises': 'error', + '@typescript-eslint/require-await': 'error', 'no-console': 'error', 'lube/svelte-naming-convention': ['error', { fixSameNames: true }], // eslint isn't that well-versed with JSDoc to know that `foo: /** @type{..} */ (foo)` isn't a violation of this rule, so turn it off @@ -43,7 +52,6 @@ export default [ 'documentation', // contains a fork of the REPL which doesn't adhere to eslint rules 'sites/svelte-5-preview/**', - 'playgrounds/demo/src/**', 'tmp/**', // wasn't checked previously, reenable at some point 'sites/svelte.dev/**' diff --git a/packages/svelte/src/internal/client/dom/elements/custom-element.js b/packages/svelte/src/internal/client/dom/elements/custom-element.js index ae966651e8c9..b6b89f813c01 100644 --- a/packages/svelte/src/internal/client/dom/elements/custom-element.js +++ b/packages/svelte/src/internal/client/dom/elements/custom-element.js @@ -193,13 +193,18 @@ if (typeof HTMLElement === 'function') { disconnectedCallback() { this.$$cn = false; // In a microtask, because this could be a move within the DOM - Promise.resolve().then(() => { - if (!this.$$cn && this.$$c) { - this.$$c.$destroy(); - destroy_effect(this.$$me); - this.$$c = undefined; - } - }); + Promise.resolve() + .then(() => { + if (!this.$$cn && this.$$c) { + this.$$c.$destroy(); + destroy_effect(this.$$me); + this.$$c = undefined; + } + }) + .catch((err) => { + // eslint-disable-next-line no-console + console.error(err); + }); } /** diff --git a/packages/svelte/src/internal/client/dom/elements/misc.js b/packages/svelte/src/internal/client/dom/elements/misc.js index 45e75c6047b6..7fbf715a86ef 100644 --- a/packages/svelte/src/internal/client/dom/elements/misc.js +++ b/packages/svelte/src/internal/client/dom/elements/misc.js @@ -42,14 +42,19 @@ export function add_form_reset_listener() { (evt) => { // Needs to happen one tick later or else the dom properties of the form // elements have not updated to their reset values yet - Promise.resolve().then(() => { - if (!evt.defaultPrevented) { - for (const e of /**@type {HTMLFormElement} */ (evt.target).elements) { - // @ts-expect-error - e.__on_r?.(); + Promise.resolve() + .then(() => { + if (!evt.defaultPrevented) { + for (const e of /**@type {HTMLFormElement} */ (evt.target).elements) { + // @ts-expect-error + e.__on_r?.(); + } } - } - }); + }) + .catch((err) => { + // eslint-disable-next-line no-console + console.error(err); + }); }, // In the capture phase to guarantee we get noticed of it (no possiblity of stopPropagation) { capture: true } diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 1f0a0bfdabac..56096c7dd3aa 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -635,7 +635,12 @@ export function schedule_effect(signal) { } else if (current_scheduler_mode === FLUSH_YIELD) { if (!is_yield_task_queued) { is_yield_task_queued = true; - yield_tick().then(process_deferred); + yield_tick() + .then(process_deferred) + .catch((err) => { + // eslint-disable-next-line no-console + console.error(err); + }); } } diff --git a/packages/svelte/src/motion/spring.js b/packages/svelte/src/motion/spring.js index ec56d354109a..1d5d7b7c652e 100644 --- a/packages/svelte/src/motion/spring.js +++ b/packages/svelte/src/motion/spring.js @@ -120,9 +120,14 @@ export function spring(value, opts = {}) { }); } return new Promise((fulfil) => { - /** @type {import('../internal/client/types').Task} */ (task).promise.then(() => { - if (token === current_token) fulfil(); - }); + /** @type {import('../internal/client/types').Task} */ (task).promise + .then(() => { + if (token === current_token) fulfil(); + }) + .catch((err) => { + // eslint-disable-next-line no-console + console.error(err); + }); }); } /** @type {import('./public.js').Spring} */ diff --git a/playgrounds/demo/.prettierignore b/playgrounds/demo/.prettierignore deleted file mode 100644 index 85de9cf93344..000000000000 --- a/playgrounds/demo/.prettierignore +++ /dev/null @@ -1 +0,0 @@ -src diff --git a/playgrounds/demo/package.json b/playgrounds/demo/package.json index cc2739cb6818..e96e8011bab5 100644 --- a/playgrounds/demo/package.json +++ b/playgrounds/demo/package.json @@ -9,9 +9,7 @@ "ssr": "node ./server.js", "build": "vite build --outDir dist/client && vite build --outDir dist/server --ssr src/entry-server.ts", "prod": "npm run build && node dist", - "preview": "vite preview", - "format": "prettier --check --write .", - "lint": "prettier --check . && eslint" + "preview": "vite preview" }, "devDependencies": { "@sveltejs/vite-plugin-svelte": "^3.1.0", diff --git a/playgrounds/sandbox/run.js b/playgrounds/sandbox/run.js index 86ec3fc882f2..05ac772af057 100644 --- a/playgrounds/sandbox/run.js +++ b/playgrounds/sandbox/run.js @@ -29,7 +29,9 @@ function mkdirp(dir) { const svelte_modules = glob('**/*.svelte', { cwd: `${cwd}/input` }); const js_modules = glob('**/*.js', { cwd: `${cwd}/input` }); -for (const generate of ['client', 'server']) { +/** @type {Array<'client'|'server'>} */ +const compile_types = ['client', 'server']; +for (const generate of compile_types) { console.error(`\n--- generating ${generate} ---\n`); for (const file of svelte_modules) { const input = `${cwd}/input/${file}`; diff --git a/playgrounds/sandbox/tsconfig.json b/playgrounds/sandbox/tsconfig.json index 44ef7add237e..8aa4b6f1e971 100644 --- a/playgrounds/sandbox/tsconfig.json +++ b/playgrounds/sandbox/tsconfig.json @@ -13,5 +13,5 @@ "allowJs": true, "checkJs": true }, - "include": ["./input"] + "include": ["./run.js", "./input"] } From 0611bef6654d25ba547e55da2b8dc4cc3a6b41d4 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Sat, 1 Jun 2024 09:29:00 -0700 Subject: [PATCH 2/9] ignore rules rather than adding catch blocks. add async to functions returning promises --- eslint.config.js | 2 ++ .../compiler/preprocess/replace_in_code.js | 2 +- .../client/dom/elements/custom-element.js | 20 ++++++++----------- .../src/internal/client/dom/elements/misc.js | 20 ++++++++----------- packages/svelte/src/motion/spring.js | 17 +++++++--------- packages/svelte/src/motion/tweened.js | 4 ++-- 6 files changed, 28 insertions(+), 37 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index 528fe94b7590..eb8c653fef8f 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -17,6 +17,8 @@ export default [ '@typescript-eslint/await-thenable': 'error', '@typescript-eslint/no-floating-promises': 'error', '@typescript-eslint/no-misused-promises': 'error', + '@typescript-eslint/prefer-promise-reject-errors': 'error', + '@typescript-eslint/promise-function-async': 'error', '@typescript-eslint/require-await': 'error', 'no-console': 'error', 'lube/svelte-naming-convention': ['error', { fixSameNames: true }], diff --git a/packages/svelte/src/compiler/preprocess/replace_in_code.js b/packages/svelte/src/compiler/preprocess/replace_in_code.js index 1439878870b1..a22b3eddd71e 100644 --- a/packages/svelte/src/compiler/preprocess/replace_in_code.js +++ b/packages/svelte/src/compiler/preprocess/replace_in_code.js @@ -20,7 +20,7 @@ export function slice_source(code_slice, offset, { file_basename, filename, get_ * @param {(...match: any[]) => Promise} get_replacement * @param {string} source */ -function calculate_replacements(re, get_replacement, source) { +async function calculate_replacements(re, get_replacement, source) { /** * @type {Array>} */ diff --git a/packages/svelte/src/internal/client/dom/elements/custom-element.js b/packages/svelte/src/internal/client/dom/elements/custom-element.js index b6b89f813c01..dc3f8fe41b7e 100644 --- a/packages/svelte/src/internal/client/dom/elements/custom-element.js +++ b/packages/svelte/src/internal/client/dom/elements/custom-element.js @@ -193,18 +193,14 @@ if (typeof HTMLElement === 'function') { disconnectedCallback() { this.$$cn = false; // In a microtask, because this could be a move within the DOM - Promise.resolve() - .then(() => { - if (!this.$$cn && this.$$c) { - this.$$c.$destroy(); - destroy_effect(this.$$me); - this.$$c = undefined; - } - }) - .catch((err) => { - // eslint-disable-next-line no-console - console.error(err); - }); + // eslint-disable-next-line @typescript-eslint/no-floating-promises + Promise.resolve().then(() => { + if (!this.$$cn && this.$$c) { + this.$$c.$destroy(); + destroy_effect(this.$$me); + this.$$c = undefined; + } + }); } /** diff --git a/packages/svelte/src/internal/client/dom/elements/misc.js b/packages/svelte/src/internal/client/dom/elements/misc.js index 7fbf715a86ef..e3745d2f5ae5 100644 --- a/packages/svelte/src/internal/client/dom/elements/misc.js +++ b/packages/svelte/src/internal/client/dom/elements/misc.js @@ -42,19 +42,15 @@ export function add_form_reset_listener() { (evt) => { // Needs to happen one tick later or else the dom properties of the form // elements have not updated to their reset values yet - Promise.resolve() - .then(() => { - if (!evt.defaultPrevented) { - for (const e of /**@type {HTMLFormElement} */ (evt.target).elements) { - // @ts-expect-error - e.__on_r?.(); - } + // eslint-disable-next-line @typescript-eslint/no-floating-promises + Promise.resolve().then(() => { + if (!evt.defaultPrevented) { + for (const e of /**@type {HTMLFormElement} */ (evt.target).elements) { + // @ts-expect-error + e.__on_r?.(); } - }) - .catch((err) => { - // eslint-disable-next-line no-console - console.error(err); - }); + } + }); }, // In the capture phase to guarantee we get noticed of it (no possiblity of stopPropagation) { capture: true } diff --git a/packages/svelte/src/motion/spring.js b/packages/svelte/src/motion/spring.js index 1d5d7b7c652e..5bd8cb89c447 100644 --- a/packages/svelte/src/motion/spring.js +++ b/packages/svelte/src/motion/spring.js @@ -77,7 +77,7 @@ export function spring(value, opts = {}) { * @param {import('./private').SpringUpdateOpts} opts * @returns {Promise} */ - function set(new_value, opts = {}) { + async function set(new_value, opts = {}) { target_value = new_value; const token = (current_token = {}); if (value == null || opts.hard || (spring.stiffness >= 1 && spring.damping >= 1)) { @@ -120,20 +120,17 @@ export function spring(value, opts = {}) { }); } return new Promise((fulfil) => { - /** @type {import('../internal/client/types').Task} */ (task).promise - .then(() => { - if (token === current_token) fulfil(); - }) - .catch((err) => { - // eslint-disable-next-line no-console - console.error(err); - }); + // eslint-disable-next-line @typescript-eslint/no-floating-promises + /** @type {import('../internal/client/types').Task} */ (task).promise.then(() => { + if (token === current_token) fulfil(); + }); }); } /** @type {import('./public.js').Spring} */ const spring = { set, - update: (fn, opts) => set(fn(/** @type {T} */ (target_value), /** @type {T} */ (value)), opts), + update: async (fn, opts) => + set(fn(/** @type {T} */ (target_value), /** @type {T} */ (value)), opts), subscribe: store.subscribe, stiffness, damping, diff --git a/packages/svelte/src/motion/tweened.js b/packages/svelte/src/motion/tweened.js index 90b6aaa0073f..9f1a8ffad659 100644 --- a/packages/svelte/src/motion/tweened.js +++ b/packages/svelte/src/motion/tweened.js @@ -88,7 +88,7 @@ export function tweened(value, defaults = {}) { * @param {T} new_value * @param {import('./private').TweenedOptions} [opts] */ - function set(new_value, opts) { + async function set(new_value, opts) { target_value = new_value; if (value == null) { @@ -145,7 +145,7 @@ export function tweened(value, defaults = {}) { } return { set, - update: (fn, opts) => + update: async (fn, opts) => set(fn(/** @type {any} */ (target_value), /** @type {any} */ (value)), opts), subscribe: store.subscribe }; From 474f3d72673ccbd670c59c6fb9502b5ac607d619 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 3 Jun 2024 17:59:49 -0400 Subject: [PATCH 3/9] remove redundant asyncs --- packages/svelte/src/compiler/preprocess/replace_in_code.js | 2 +- packages/svelte/src/motion/spring.js | 5 ++--- packages/svelte/src/motion/tweened.js | 4 ++-- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/svelte/src/compiler/preprocess/replace_in_code.js b/packages/svelte/src/compiler/preprocess/replace_in_code.js index a22b3eddd71e..1439878870b1 100644 --- a/packages/svelte/src/compiler/preprocess/replace_in_code.js +++ b/packages/svelte/src/compiler/preprocess/replace_in_code.js @@ -20,7 +20,7 @@ export function slice_source(code_slice, offset, { file_basename, filename, get_ * @param {(...match: any[]) => Promise} get_replacement * @param {string} source */ -async function calculate_replacements(re, get_replacement, source) { +function calculate_replacements(re, get_replacement, source) { /** * @type {Array>} */ diff --git a/packages/svelte/src/motion/spring.js b/packages/svelte/src/motion/spring.js index 5bd8cb89c447..c98a2fc47068 100644 --- a/packages/svelte/src/motion/spring.js +++ b/packages/svelte/src/motion/spring.js @@ -77,7 +77,7 @@ export function spring(value, opts = {}) { * @param {import('./private').SpringUpdateOpts} opts * @returns {Promise} */ - async function set(new_value, opts = {}) { + function set(new_value, opts = {}) { target_value = new_value; const token = (current_token = {}); if (value == null || opts.hard || (spring.stiffness >= 1 && spring.damping >= 1)) { @@ -129,8 +129,7 @@ export function spring(value, opts = {}) { /** @type {import('./public.js').Spring} */ const spring = { set, - update: async (fn, opts) => - set(fn(/** @type {T} */ (target_value), /** @type {T} */ (value)), opts), + update: (fn, opts) => set(fn(/** @type {T} */ (target_value), /** @type {T} */ (value)), opts), subscribe: store.subscribe, stiffness, damping, diff --git a/packages/svelte/src/motion/tweened.js b/packages/svelte/src/motion/tweened.js index 9f1a8ffad659..90b6aaa0073f 100644 --- a/packages/svelte/src/motion/tweened.js +++ b/packages/svelte/src/motion/tweened.js @@ -88,7 +88,7 @@ export function tweened(value, defaults = {}) { * @param {T} new_value * @param {import('./private').TweenedOptions} [opts] */ - async function set(new_value, opts) { + function set(new_value, opts) { target_value = new_value; if (value == null) { @@ -145,7 +145,7 @@ export function tweened(value, defaults = {}) { } return { set, - update: async (fn, opts) => + update: (fn, opts) => set(fn(/** @type {any} */ (target_value), /** @type {any} */ (value)), opts), subscribe: store.subscribe }; From 3f2dd19cda636ef1d6cf189cf372fd66d501c57f Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 3 Jun 2024 18:01:10 -0400 Subject: [PATCH 4/9] remove extremely pointless rule --- eslint.config.js | 1 - 1 file changed, 1 deletion(-) diff --git a/eslint.config.js b/eslint.config.js index eb8c653fef8f..b28e0e8661ca 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -18,7 +18,6 @@ export default [ '@typescript-eslint/no-floating-promises': 'error', '@typescript-eslint/no-misused-promises': 'error', '@typescript-eslint/prefer-promise-reject-errors': 'error', - '@typescript-eslint/promise-function-async': 'error', '@typescript-eslint/require-await': 'error', 'no-console': 'error', 'lube/svelte-naming-convention': ['error', { fixSameNames: true }], From 6adfd8468b79cfb6d05e76c06f15a3561fcaa77d Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 3 Jun 2024 18:03:10 -0400 Subject: [PATCH 5/9] remove another daft rule --- eslint.config.js | 1 - .../svelte/src/internal/client/dom/elements/custom-element.js | 1 - packages/svelte/src/internal/client/dom/elements/misc.js | 1 - packages/svelte/src/motion/spring.js | 1 - 4 files changed, 4 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index b28e0e8661ca..d0afc51898db 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -15,7 +15,6 @@ export default [ }, rules: { '@typescript-eslint/await-thenable': 'error', - '@typescript-eslint/no-floating-promises': 'error', '@typescript-eslint/no-misused-promises': 'error', '@typescript-eslint/prefer-promise-reject-errors': 'error', '@typescript-eslint/require-await': 'error', diff --git a/packages/svelte/src/internal/client/dom/elements/custom-element.js b/packages/svelte/src/internal/client/dom/elements/custom-element.js index dc3f8fe41b7e..ae966651e8c9 100644 --- a/packages/svelte/src/internal/client/dom/elements/custom-element.js +++ b/packages/svelte/src/internal/client/dom/elements/custom-element.js @@ -193,7 +193,6 @@ if (typeof HTMLElement === 'function') { disconnectedCallback() { this.$$cn = false; // In a microtask, because this could be a move within the DOM - // eslint-disable-next-line @typescript-eslint/no-floating-promises Promise.resolve().then(() => { if (!this.$$cn && this.$$c) { this.$$c.$destroy(); diff --git a/packages/svelte/src/internal/client/dom/elements/misc.js b/packages/svelte/src/internal/client/dom/elements/misc.js index e3745d2f5ae5..45e75c6047b6 100644 --- a/packages/svelte/src/internal/client/dom/elements/misc.js +++ b/packages/svelte/src/internal/client/dom/elements/misc.js @@ -42,7 +42,6 @@ export function add_form_reset_listener() { (evt) => { // Needs to happen one tick later or else the dom properties of the form // elements have not updated to their reset values yet - // eslint-disable-next-line @typescript-eslint/no-floating-promises Promise.resolve().then(() => { if (!evt.defaultPrevented) { for (const e of /**@type {HTMLFormElement} */ (evt.target).elements) { diff --git a/packages/svelte/src/motion/spring.js b/packages/svelte/src/motion/spring.js index c98a2fc47068..ec56d354109a 100644 --- a/packages/svelte/src/motion/spring.js +++ b/packages/svelte/src/motion/spring.js @@ -120,7 +120,6 @@ export function spring(value, opts = {}) { }); } return new Promise((fulfil) => { - // eslint-disable-next-line @typescript-eslint/no-floating-promises /** @type {import('../internal/client/types').Task} */ (task).promise.then(() => { if (token === current_token) fulfil(); }); From 5ef72906a1d2acdb7ea2ddd8b349debdb0c7c734 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 3 Jun 2024 18:04:12 -0400 Subject: [PATCH 6/9] this is what typescript is for --- eslint.config.js | 1 - 1 file changed, 1 deletion(-) diff --git a/eslint.config.js b/eslint.config.js index d0afc51898db..3ba1ae105ff4 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -14,7 +14,6 @@ export default [ lube }, rules: { - '@typescript-eslint/await-thenable': 'error', '@typescript-eslint/no-misused-promises': 'error', '@typescript-eslint/prefer-promise-reject-errors': 'error', '@typescript-eslint/require-await': 'error', From 843a9ae6d5e3b3843d3d9180364da65422d31439 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 3 Jun 2024 18:05:11 -0400 Subject: [PATCH 7/9] again, typescript already has this covered, we don't need it --- eslint.config.js | 1 - 1 file changed, 1 deletion(-) diff --git a/eslint.config.js b/eslint.config.js index 3ba1ae105ff4..595d43116bea 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -14,7 +14,6 @@ export default [ lube }, rules: { - '@typescript-eslint/no-misused-promises': 'error', '@typescript-eslint/prefer-promise-reject-errors': 'error', '@typescript-eslint/require-await': 'error', 'no-console': 'error', From e22befd8e0a42e73f45db55dda85c46fd592f486 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 3 Jun 2024 18:09:05 -0400 Subject: [PATCH 8/9] simplify --- playgrounds/sandbox/run.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/playgrounds/sandbox/run.js b/playgrounds/sandbox/run.js index 05ac772af057..dffcc0af58a2 100644 --- a/playgrounds/sandbox/run.js +++ b/playgrounds/sandbox/run.js @@ -29,9 +29,7 @@ function mkdirp(dir) { const svelte_modules = glob('**/*.svelte', { cwd: `${cwd}/input` }); const js_modules = glob('**/*.js', { cwd: `${cwd}/input` }); -/** @type {Array<'client'|'server'>} */ -const compile_types = ['client', 'server']; -for (const generate of compile_types) { +for (const generate of /** @type {const} */ (['client', 'server'])) { console.error(`\n--- generating ${generate} ---\n`); for (const file of svelte_modules) { const input = `${cwd}/input/${file}`; From 7d5f7bb522ea5264372d9eafd73f995944a68db2 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 4 Jun 2024 15:13:31 -0400 Subject: [PATCH 9/9] this rule is harmless --- eslint.config.js | 1 + 1 file changed, 1 insertion(+) diff --git a/eslint.config.js b/eslint.config.js index 595d43116bea..d8875d37f9a1 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -14,6 +14,7 @@ export default [ lube }, rules: { + '@typescript-eslint/await-thenable': 'error', '@typescript-eslint/prefer-promise-reject-errors': 'error', '@typescript-eslint/require-await': 'error', 'no-console': 'error',