Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: cleanup promise handling #12284

Closed
wants to merge 16 commits into from
Closed

chore: cleanup promise handling #12284

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
c00b1f3
chore: cleanup promise handling
benmccann May 31, 2024
0d23637
revert breaking changes
Rich-Harris Jun 3, 2024
88eeab6
revert
Rich-Harris Jun 3, 2024
6f5b2d8
fix
Rich-Harris Jun 3, 2024
c4c2175
Update packages/kit/src/runtime/client/fetcher.js
Rich-Harris Jun 3, 2024
5512a5a
appease the nanny
Rich-Harris Jun 3, 2024
13ee5f8
Merge branch 'promise-cleanup' of github.com:sveltejs/kit into promis…
Rich-Harris Jun 3, 2024
8874d96
another
Rich-Harris Jun 3, 2024
336b639
remove need for an ignore
benmccann Jun 4, 2024
743c640
address some no-floating-promises issues
benmccann Jun 4, 2024
24fc701
format
benmccann Jun 4, 2024
f156eaa
load in parallel
benmccann Jun 4, 2024
e244c28
enqueue in parallel
benmccann Jun 4, 2024
e075503
format
benmccann Jun 4, 2024
f9e02f4
fix stupid mistake
benmccann Jun 4, 2024
dd553bc
revert check_stack_trace change
benmccann Jun 4, 2024
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
22 changes: 21 additions & 1 deletion eslint.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,17 @@ import svelte_config from '@sveltejs/eslint-config';
export default [
...svelte_config,
{
languageOptions: {
parserOptions: {
project: true
}
},
rules: {
'@typescript-eslint/await-thenable': 'error',
// '@typescript-eslint/no-floating-promises': 'error',
// '@typescript-eslint/no-misused-promises': 'error',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a look at the errors that arise if this rule is enabled, and not a single one is valid. It's just ESLint whining pointlessly

Suggested change
// '@typescript-eslint/no-misused-promises': 'error',

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a lot more errors pop up with no-floating-promises and i haven't looked at them all yet, but of the ones i have looked at it's the same story. definitely not something we want

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why the issues no-floating-promises is identifying shouldn't be addressed. I just pushed a commit addressing a lot of them: 743c640. We can drop it if they're really not issues. This isn't my strong point, so it's quite possible I'm missing something. I figured I'd push it though so that I can at least learn what that is as it's easier to discuss that way.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're really not issues, but the rule tricked you into breaking code that works perfectly well

'@typescript-eslint/no-unused-expressions': 'off',
'@typescript-eslint/require-await': 'error',
'no-undef': 'off'
}
},
Expand All @@ -15,7 +24,18 @@ export default [
'packages/adapter-static/test/apps/*/build',
'packages/adapter-cloudflare/files',
'packages/adapter-netlify/files',
'packages/adapter-node/files'
'packages/adapter-node/files',
'packages/adapter-node/rollup.config.js',
'packages/adapter-node/tests/smoke.spec.js',
'packages/adapter-static/test/apps',
'packages/create-svelte/shared',
'packages/create-svelte/templates',
'packages/kit/src/core/sync/create_manifest_data/test/samples',
'packages/kit/test/apps',
'packages/kit/test/build-errors',
'packages/kit/test/prerendering',
'packages/package/test/errors',
'packages/package/test/fixtures'
]
}
];
4 changes: 2 additions & 2 deletions packages/adapter-auto/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ function detect_package_manager() {
}

/** @param {string} name */
async function import_from_cwd(name) {
function import_from_cwd(name) {
const cwd = pathToFileURL(process.cwd()).href;
const url = await resolve(name, cwd + '/x.js');
const url = resolve(name, cwd + '/x.js');

return import(url);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/adapter-netlify/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export default function ({ split = false, edge = edge_set_in_env_var } = {}) {

await generate_edge_functions({ builder });
} else {
await generate_lambda_functions({ builder, split, publish });
generate_lambda_functions({ builder, split, publish });
}
},

Expand Down Expand Up @@ -182,7 +182,7 @@ async function generate_edge_functions({ builder }) {
* @param { string } params.publish
* @param { boolean } params.split
*/
async function generate_lambda_functions({ builder, publish, split }) {
function generate_lambda_functions({ builder, publish, split }) {
builder.mkdirp('.netlify/functions-internal/.svelte-kit');

/** @type {string[]} */
Expand Down
4 changes: 2 additions & 2 deletions packages/enhanced-img/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
/**
* @returns {Promise<import('vite').Plugin[]>}
*/
export async function enhancedImages() {

Check failure on line 8 in packages/enhanced-img/src/index.js

View workflow job for this annotation

GitHub Actions / lint-all 

Async function 'enhancedImages' has no 'await' expression
const imagetools_instance = await imagetools_plugin();
const imagetools_instance = imagetools_plugin();
return !process.versions.webcontainer
? [image_plugin(imagetools_instance), imagetools_instance]
: [];
Expand Down Expand Up @@ -60,7 +60,7 @@
'.webp': 'png'
};

async function imagetools_plugin() {
function imagetools_plugin() {
/** @type {Partial<import('vite-imagetools').VitePluginOptions>} */
const imagetools_opts = {
defaultDirectives: async ({ pathname, searchParams: qs }, metadata) => {
Expand Down
2 changes: 1 addition & 1 deletion packages/kit/postinstall.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ try {

try {
const config = await load_config();
await sync.all(config, 'development');
sync.all(config, 'development');
} catch (error) {
console.error('Error while trying to sync SvelteKit config');
console.error(error);
Expand Down
2 changes: 1 addition & 1 deletion packages/kit/src/cli.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ prog
try {
const config = await load_config();
const sync = await import('./core/sync/sync.js');
await sync.all_types(config, mode);
sync.all_types(config, mode);
} catch (error) {
handle_error(error);
}
Expand Down
16 changes: 8 additions & 8 deletions packages/kit/src/core/sync/sync.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ export function init(config, mode) {
* Update SvelteKit's generated files
* @param {import('types').ValidatedConfig} config
*/
export async function create(config) {
export function create(config) {
const manifest_data = create_manifest_data({ config });

const output = path.join(config.kit.outDir, 'generated');

write_client_manifest(config.kit, manifest_data, `${output}/client`);
write_server(config, output);
write_root(manifest_data, output);
await write_all_types(config, manifest_data);
write_all_types(config, manifest_data);

return { manifest_data };
}
Expand All @@ -44,8 +44,8 @@ export async function create(config) {
* @param {import('types').ManifestData} manifest_data
* @param {string} file
*/
export async function update(config, manifest_data, file) {
await write_types(config, manifest_data, file);
export function update(config, manifest_data, file) {
write_types(config, manifest_data, file);

return { manifest_data };
}
Expand All @@ -55,20 +55,20 @@ export async function update(config, manifest_data, file) {
* @param {import('types').ValidatedConfig} config
* @param {string} mode The Vite mode
*/
export async function all(config, mode) {
export function all(config, mode) {
init(config, mode);
return await create(config);
return create(config);
}

/**
* Run sync.init and then generate all type files.
* @param {import('types').ValidatedConfig} config
* @param {string} mode The Vite mode
*/
export async function all_types(config, mode) {
export function all_types(config, mode) {
init(config, mode);
const manifest_data = create_manifest_data({ config });
await write_all_types(config, manifest_data);
write_all_types(config, manifest_data);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions packages/kit/src/core/sync/write_types/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const cwd = process.cwd();
* @param {import('types').ValidatedConfig} config
* @param {import('types').ManifestData} manifest_data
*/
export async function write_all_types(config, manifest_data) {
export function write_all_types(config, manifest_data) {
if (!ts) return;

const types_dir = `${config.kit.outDir}/types`;
Expand Down Expand Up @@ -133,7 +133,7 @@ export async function write_all_types(config, manifest_data) {
* @param {import('types').ManifestData} manifest_data
* @param {string} file
*/
export async function write_types(config, manifest_data, file) {
export function write_types(config, manifest_data, file) {
if (!ts) return;

if (!path.basename(file).startsWith('+')) {
Expand Down
24 changes: 12 additions & 12 deletions packages/kit/src/core/sync/write_types/index.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const cwd = fileURLToPath(new URL('./test', import.meta.url));
/**
* @param {string} dir
*/
async function run_test(dir) {
function run_test(dir) {
rimraf(path.join(cwd, dir, '.svelte-kit'));

const initial = options({}, 'config');
Expand All @@ -25,22 +25,22 @@ async function run_test(dir) {
const manifest = create_manifest_data({
config: /** @type {import('types').ValidatedConfig} */ (initial)
});
await write_all_types(initial, manifest);
write_all_types(initial, manifest);
}

test('Creates correct $types', async () => {
test('Creates correct $types', () => {
// To save us from creating a real SvelteKit project for each of the tests,
// we first run the type generation directly for each test case, and then
// call `tsc` to check that the generated types are valid.
await run_test('actions');
await run_test('simple-page-shared-only');
await run_test('simple-page-server-only');
await run_test('simple-page-server-and-shared');
await run_test('layout');
await run_test('layout-advanced');
await run_test('slugs');
await run_test('slugs-layout-not-all-pages-have-load');
await run_test('param-type-inference');
run_test('actions');
run_test('simple-page-shared-only');
run_test('simple-page-server-only');
run_test('simple-page-server-and-shared');
run_test('layout');
run_test('layout-advanced');
run_test('slugs');
run_test('slugs-layout-not-all-pages-have-load');
run_test('param-type-inference');
try {
execSync('pnpm testtypes', { cwd });
} catch (e) {
Expand Down
4 changes: 2 additions & 2 deletions packages/kit/src/exports/hooks/sequence.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ test('uses first defined preload option', async () => {
const event = /** @type {import('@sveltejs/kit').RequestEvent} */ ({});
const response = await handler({
event,
resolve: async (_event, opts = {}) => {
resolve: (_event, opts = {}) => {
let html = '';

const { preload = () => false } = opts;
Expand Down Expand Up @@ -153,7 +153,7 @@ test('uses first defined filterSerializedResponseHeaders option', async () => {
const event = /** @type {import('@sveltejs/kit').RequestEvent} */ ({});
const response = await handler({
event,
resolve: async (_event, opts = {}) => {
resolve: (_event, opts = {}) => {
let html = '';

const { filterSerializedResponseHeaders = () => false } = opts;
Expand Down
4 changes: 2 additions & 2 deletions packages/kit/src/exports/vite/build/build_service_worker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,13 @@ export async function build_service_worker(
*/
const sw_virtual_modules = {
name: 'service-worker-build-virtual-modules',
async resolveId(id) {
resolveId(id) {
if (id.startsWith('$env/') || id.startsWith('$app/') || id === '$service-worker') {
return `\0virtual:${id}`;
}
},

async load(id) {
load(id) {
if (!id.startsWith('\0virtual:')) return;

if (id === service_worker) {
Expand Down
8 changes: 4 additions & 4 deletions packages/kit/src/exports/vite/dev/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,9 @@ export async function dev(vite, vite_config, svelte_config) {
return { module, module_node, url };
}

async function update_manifest() {
function update_manifest() {
try {
({ manifest_data } = await sync.create(svelte_config));
({ manifest_data } = sync.create(svelte_config));

if (manifest_error) {
manifest_error = null;
Expand Down Expand Up @@ -273,7 +273,7 @@ export async function dev(vite, vite_config, svelte_config) {
return error.stack;
}

await update_manifest();
update_manifest();

/**
* @param {string} event
Expand Down Expand Up @@ -389,7 +389,7 @@ export async function dev(vite, vite_config, svelte_config) {
return ws_send.apply(vite.ws, args);
};

vite.middlewares.use(async (req, res, next) => {
vite.middlewares.use((req, res, next) => {
try {
const base = `${vite.config.server.https ? 'https' : 'http'}://${
req.headers[':authority'] || req.headers.host
Expand Down
14 changes: 7 additions & 7 deletions packages/kit/src/exports/vite/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ const warning_preprocessor = {
async function resolve_peer_dependency(dependency) {
try {
// @ts-expect-error the types are wrong
const resolved = await imr.resolve(dependency, pathToFileURL(process.cwd() + '/dummy.js'));
const resolved = imr.resolve(dependency, pathToFileURL(process.cwd() + '/dummy.js'));
return import(resolved);
} catch {
throw new Error(
Expand Down Expand Up @@ -242,7 +242,7 @@ async function kit({ svelte_config }) {
* Build the SvelteKit-provided Vite config to be merged with the user's vite.config.js file.
* @see https://vitejs.dev/guide/api-plugin.html#config
*/
async config(config, config_env) {
config(config, config_env) {
initial_config = config;
vite_config_env = config_env;
is_build = config_env.command === 'build';
Expand Down Expand Up @@ -339,7 +339,7 @@ async function kit({ svelte_config }) {
};

if (!secondary_build_started) {
manifest_data = (await sync.all(svelte_config, config_env.mode)).manifest_data;
manifest_data = sync.all(svelte_config, config_env.mode).manifest_data;
}
} else {
new_config.define = {
Expand Down Expand Up @@ -373,7 +373,7 @@ async function kit({ svelte_config }) {
const plugin_virtual_modules = {
name: 'vite-plugin-sveltekit-virtual-modules',

async resolveId(id, importer) {
resolveId(id, importer) {
// If importing from a service-worker, only allow $service-worker & $env/static/public, but none of the other virtual modules.
// This check won't catch transitive imports, but it will warn when the import comes from a service-worker directly.
// Transitive imports will be caught during the build.
Expand All @@ -397,7 +397,7 @@ async function kit({ svelte_config }) {
}
},

async load(id, options) {
load(id, options) {
const browser = !options?.ssr;

const global = is_build
Expand Down Expand Up @@ -533,7 +533,7 @@ async function kit({ svelte_config }) {

writeBundle: {
sequential: true,
async handler(_options) {
handler(_options) {
if (vite_config.build.ssr) return;

const guard = module_guard(this, {
Expand All @@ -560,7 +560,7 @@ async function kit({ svelte_config }) {
* Build the SvelteKit-provided Vite config to be merged with the user's vite.config.js file.
* @see https://vitejs.dev/guide/api-plugin.html#config
*/
async config(config) {
config(config) {
/** @type {import('vite').UserConfig} */
let new_config;

Expand Down
Loading
Loading