From e3891e91533ded8f85dbb8eb51e984930037c9ca Mon Sep 17 00:00:00 2001 From: Felix Kling Date: Wed, 3 Apr 2024 12:00:57 +0200 Subject: [PATCH] svelte/ci: Run `svelte-check` in CI (#61527) This commit updates the bazel build file to execute `svelte-check` in CI. To make this work correctly we need to run additional commands to generate all the files that are needed by the code. Specifically that's `svelte-kit sync` and the graphql code/type generation. The code was refactored to make that work. This commit also fixes issues detected by `svelte-check`. IMPORTANT Running `svelte-check` in CI differs in two ways from running it locally: 1. Locally `svelte-check` will also check all local `@sourcegraph/*`, because they symlink the orginal source code. In CI/Bazel those dependencies are already built and therefore are not checked by Bazel. However, this is what we want because the dependencies do not always confirm to the rules set by `svelte-check`. 2. Source files that are not required for the production build have been excluded and are therefore not checked. That includes Storybook story files, playwright and vitest files and `src/testing/*`. --- client/web-sveltekit/BUILD.bazel | 131 +++++++++++-- client/web-sveltekit/dev/generate-graphql.ts | 7 + .../web-sveltekit/dev/vite-graphql-codegen.ts | 181 +++++++++--------- client/web-sveltekit/src/lib/graphql/urql.ts | 3 +- .../src/lib/wildcard/Button.svelte | 8 +- .../src/lib/wildcard/menu/Menu.svelte | 4 +- client/web-sveltekit/src/routes/layout.gql | 4 +- client/web-sveltekit/vite.config.ts | 6 +- 8 files changed, 233 insertions(+), 111 deletions(-) create mode 100644 client/web-sveltekit/dev/generate-graphql.ts diff --git a/client/web-sveltekit/BUILD.bazel b/client/web-sveltekit/BUILD.bazel index 4a46dddabd4..1d73dd056dd 100644 --- a/client/web-sveltekit/BUILD.bazel +++ b/client/web-sveltekit/BUILD.bazel @@ -1,8 +1,14 @@ load("@bazel_skylib//rules:build_test.bzl", "build_test") +load("@bazel_skylib//lib:partial.bzl", "partial") load("@npm//:defs.bzl", "npm_link_all_packages") load("@npm//client/web-sveltekit:vite/package_json.bzl", vite_bin = "bin") load("@npm//client/web-sveltekit:vitest/package_json.bzl", vitest_bin = "bin") -load("//dev:defs.bzl", "vitest_test") +load("@npm//client/web-sveltekit:svelte-check/package_json.bzl", svelte_check = "bin") +load("@npm//client/web-sveltekit:@sveltejs/kit/package_json.bzl", sveltekit = "bin") +load("@aspect_rules_ts//ts:defs.bzl", "ts_config", "ts_project") +load("@aspect_rules_js//js:defs.bzl", "js_binary", "js_run_binary") +load("@aspect_rules_swc//swc:defs.bzl", "swc") +load("//dev:defs.bzl", "ts_binary", "vitest_test") # gazelle:ignore @@ -32,18 +38,18 @@ SRCS = [ [ "src/lib/graphql-operations.ts", "src/lib/graphql-types.ts", - "src/lib/graphql-type-mocks.ts", + "src/testing/graphql-type-mocks.ts", "src/**/*.gql.ts", "src/**/*.gql.d.ts", "src/**/*.test.ts", "src/**/*.spec.ts", + "src/testing/*", + "src/**/*.stories.svelte", ], -) + glob([ - "dev/**/*.cjs", - "dev/**/*.ts", -]) +) BUILD_DEPS = [ + "//cmd/frontend/graphqlbackend:graphql_schema", "//:node_modules/@apollo/client", "//:node_modules/@codemirror/autocomplete", "//:node_modules/@codemirror/commands", @@ -64,8 +70,6 @@ BUILD_DEPS = [ "//:node_modules/react-resizable", "//:node_modules/rxjs", "//:node_modules/uuid", - "//cmd/frontend/graphqlbackend:graphql_schema", - ":node_modules/hotkeys-js", ":node_modules/@faker-js/faker", ":node_modules/@floating-ui/dom", ":node_modules/@graphql-codegen/cli", @@ -76,6 +80,7 @@ BUILD_DEPS = [ ":node_modules/@graphql-tools/utils", ":node_modules/@melt-ui/svelte", ":node_modules/@sourcegraph/branded", + ":node_modules/@sourcegraph/client-api", ":node_modules/@sourcegraph/common", ":node_modules/@sourcegraph/http-client", ":node_modules/@sourcegraph/shared", @@ -88,14 +93,19 @@ BUILD_DEPS = [ ":node_modules/@types/prismjs", ":node_modules/@urql/core", ":node_modules/graphql", + ":node_modules/hotkeys-js", ":node_modules/prismjs", ":node_modules/sass", + ":node_modules/signale", ":node_modules/svelte", ":node_modules/ts-key-enum", ":node_modules/vite", ":node_modules/vite-plugin-inspect", ":node_modules/wonka", -] +] + glob([ + "dev/**/*.cjs", + "dev/**/*.ts", +]) CONFIGS = [ "//client/branded:tsconfig", @@ -138,11 +148,19 @@ TESTS = glob([ ]) TEST_BUILD_DEPS = [ + ":generate-graphql-types", ":node_modules/vitest", - ":node_modules/signale", ":node_modules/@testing-library/svelte", ":node_modules/@testing-library/user-event", -] +] + glob( + [ + "src/testing/*", + ], + [ + # Already inluded in TESTS + "src/testing/graphql-mocking.test.ts", + ], +) vitest_test( name = "unit_tests", @@ -151,3 +169,94 @@ vitest_test( data = SRCS + BUILD_DEPS + CONFIGS + TESTS + TEST_BUILD_DEPS, with_vitest_config = False, ) + +# Create binary that executes graphql-codegen +ts_binary( + name = "graphql_codegen_bin", + data = [ + "dev/graphql-type-mocks.cjs", + "dev/vite-graphql-codegen.ts", + ":node_modules/@graphql-codegen/cli", + ":node_modules/@graphql-codegen/near-operation-file-preset", + ":node_modules/@graphql-codegen/typed-document-node", + ":node_modules/@graphql-codegen/typescript", + ":node_modules/@graphql-codegen/typescript-operations", + ":node_modules/@graphql-tools/utils", + ":node_modules/@graphql-typed-document-node/core", + ":node_modules/graphql", + ":node_modules/signale", + ], + entry_point = "dev/generate-graphql.ts", +) + +# Generate types from graphql files +GRAPHQL_FILES = glob( + [ + "src/**/*.gql", + ], +) + +# Files that possibly contain gql`...` tags +OTHER_GRAPHQL_INPUT_FILES = glob( + [ + "src/**/*.ts", + ], + [ + "src/lib/graphql-*.ts", + "src/testing/graphql-type-mocks.ts", + "src/**/*.gql.ts", + ], +) + +# Run graphql-codegen +js_run_binary( + name = "generate-graphql-types", + srcs = GRAPHQL_FILES + OTHER_GRAPHQL_INPUT_FILES + [ + "//cmd/frontend/graphqlbackend:graphql_schema", + ], + outs = [src + ".ts" for src in GRAPHQL_FILES] + [ + "src/lib/graphql-operations.ts", + "src/lib/graphql-types.ts", + "src/testing/graphql-type-mocks.ts", + ], + chdir = package_name(), + tool = ":graphql_codegen_bin", +) + +# Generate SvelteKit specific types +sveltekit.svelte_kit( + name = "sveltekit-sync", + srcs = SRCS + [ + ":node_modules/@sveltejs/adapter-static", + ":node_modules/@sveltejs/vite-plugin-svelte", + # This is needed to sveltekit sync to generate the types + # It explicitly looks for the existince of this package + "//:node_modules/typescript", + ], + args = [ + "sync", + ], + chdir = package_name(), + out_dirs = [ + ".svelte-kit", + ], +) + +# Runs svelte-check on production source files +svelte_check.svelte_check_test( + name = "svelte-check", + timeout = "short", + args = [ + "--tsconfig", + "tsconfig.json", + ], + chdir = package_name(), + data = SRCS + BUILD_DEPS + CONFIGS + [ + ":sveltekit-sync", + ":generate-graphql-types", + ":node_modules/@graphql-typed-document-node/core", + "//:node_modules/@types/uuid", + # Needed to properly extend vite's UserConfig type + ":node_modules/vitest", + ], +) diff --git a/client/web-sveltekit/dev/generate-graphql.ts b/client/web-sveltekit/dev/generate-graphql.ts new file mode 100644 index 00000000000..1402a52ab87 --- /dev/null +++ b/client/web-sveltekit/dev/generate-graphql.ts @@ -0,0 +1,7 @@ +// Standalone script to generate graphql types, used by bazel +import { codegen } from './vite-graphql-codegen' + +codegen().catch(error => { + console.error(error) + process.exit(1) +}) diff --git a/client/web-sveltekit/dev/vite-graphql-codegen.ts b/client/web-sveltekit/dev/vite-graphql-codegen.ts index 2f1d964eae7..f8a52b5178e 100644 --- a/client/web-sveltekit/dev/vite-graphql-codegen.ts +++ b/client/web-sveltekit/dev/vite-graphql-codegen.ts @@ -1,10 +1,96 @@ -import { dirname } from 'node:path' -import { fileURLToPath } from 'node:url' - import { generate, type CodegenConfig } from '@graphql-codegen/cli' import type { Plugin } from 'vite' -const __dirname = dirname(fileURLToPath(import.meta.url)) +const codgegenConfig: CodegenConfig = { + hooks: { + // This hook removes the 'import type * as Types from ...' import from generated files if it's not used. + // The near-operation-file preset generates this import for every file, even if it's not used. This + // generally is not a problem, but the issue is reported by `pnpm check`. + // See https://github.com/dotansimha/graphql-code-generator/issues/4900 + beforeOneFileWrite(_path, content) { + if (/^import type \* as Types from/m.test(content) && !/Types(\[|\.)/.test(content)) { + return content.replace(/^import type \* as Types from .+$/m, '').trimStart() + } + return content + }, + }, + generates: { + // Legacy graphql-operations.ts file that is still used by some components. + './src/lib/graphql-operations.ts': { + documents: [ + 'src/{lib,routes}/**/*.ts', + '!src/lib/graphql-{operations,types,type-mocks}.ts', + '!src/**/*.gql.ts', + ], + config: { + onlyOperationTypes: true, + enumValues: '$lib/graphql-types', + }, + plugins: ['typescript', 'typescript-operations'], + }, + 'src/lib/graphql-types.ts': { + plugins: ['typescript'], + }, + 'src/testing/graphql-type-mocks.ts': { + documents: [ + 'src/{lib,routes}/**/*.(ts|gql)', + '!src/lib/graphql-{operations,types,type-mocks}.ts', + '!src/**/*.gql.ts', + ], + config: { + typesImport: '$lib/graphql-types', + onlyOperationTypes: true, + }, + plugins: [`typescript`, `typescript-operations`, `./dev/graphql-type-mocks.cjs`], + }, + 'src/': { + documents: ['src/**/*.gql', '!src/**/*.gql.ts'], + preset: 'near-operation-file', + presetConfig: { + baseTypesPath: 'lib/graphql-types', + extension: '.gql.ts', + }, + config: { + useTypeImports: true, + documentVariableSuffix: '', // The default is 'Document' + }, + plugins: ['typescript-operations', 'typed-document-node'], + }, + }, + schema: '../../cmd/frontend/graphqlbackend/*.graphql', + errorsOnly: true, + config: { + // https://the-guild.dev/graphql/codegen/plugins/typescript/typescript-operations#config-api-reference + arrayInputCoercion: false, + preResolveTypes: true, + operationResultSuffix: 'Result', + omitOperationSuffix: true, + namingConvention: { + typeNames: 'keep', + enumValues: 'keep', + transformUnderscore: true, + }, + declarationKind: 'interface', + avoidOptionals: { + field: true, + inputValue: false, + object: true, + }, + scalars: { + DateTime: 'string', + JSON: 'object', + JSONValue: 'unknown', + GitObjectID: 'string', + JSONCString: 'string', + PublishedValue: "boolean | 'draft'", + BigInt: 'string', + }, + }, +} + +export function codegen(): Promise { + return generate(codgegenConfig, true) +} /** * Vite plugin for generating TypeScript types for GraphQL. @@ -13,93 +99,6 @@ const __dirname = dirname(fileURLToPath(import.meta.url)) * watch mode when documents are defined separately for every generated file. */ export default function graphqlCodegen(): Plugin { - const codgegenConfig: CodegenConfig = { - hooks: { - // This hook removes the 'import type * as Types from ...' import from generated files if it's not used. - // The near-operation-file preset generates this import for every file, even if it's not used. This - // generally is not a problem, but the issue is reported by `pnpm check`. - // See https://github.com/dotansimha/graphql-code-generator/issues/4900 - beforeOneFileWrite(_path, content) { - if (/^import type \* as Types from/m.test(content) && !/Types(\[|\.)/.test(content)) { - return content.replace(/^import type \* as Types from .+$/m, '').trimStart() - } - return content - }, - }, - generates: { - // Legacy graphql-operations.ts file that is still used by some components. - './src/lib/graphql-operations.ts': { - documents: [ - 'src/{lib,routes}/**/*.ts', - '!src/lib/graphql-{operations,types,type-mocks}.ts', - '!src/**/*.gql.ts', - ], - config: { - onlyOperationTypes: true, - enumValues: '$lib/graphql-types', - }, - plugins: ['typescript', 'typescript-operations'], - }, - 'src/lib/graphql-types.ts': { - plugins: ['typescript'], - }, - 'src/testing/graphql-type-mocks.ts': { - documents: [ - 'src/{lib,routes}/**/*.(ts|gql)', - '!src/lib/graphql-{operations,types,type-mocks}.ts', - '!src/**/*.gql.ts', - ], - config: { - typesImport: '$lib/graphql-types', - onlyOperationTypes: true, - }, - plugins: [`typescript`, `typescript-operations`, `${__dirname}/graphql-type-mocks.cjs`], - }, - 'src/': { - documents: ['src/**/*.gql', '!src/**/*.gql.ts'], - preset: 'near-operation-file', - presetConfig: { - baseTypesPath: 'lib/graphql-types', - extension: '.gql.ts', - }, - config: { - useTypeImports: true, - documentVariableSuffix: '', // The default is 'Document' - }, - plugins: ['typescript-operations', 'typed-document-node'], - }, - }, - schema: '../../cmd/frontend/graphqlbackend/*.graphql', - errorsOnly: true, - config: { - // https://the-guild.dev/graphql/codegen/plugins/typescript/typescript-operations#config-api-reference - arrayInputCoercion: false, - preResolveTypes: true, - operationResultSuffix: 'Result', - omitOperationSuffix: true, - namingConvention: { - typeNames: 'keep', - enumValues: 'keep', - transformUnderscore: true, - }, - declarationKind: 'interface', - avoidOptionals: { - field: true, - inputValue: false, - object: true, - }, - scalars: { - DateTime: 'string', - JSON: 'object', - JSONValue: 'unknown', - GitObjectID: 'string', - JSONCString: 'string', - PublishedValue: "boolean | 'draft'", - BigInt: 'string', - }, - }, - } - async function codegen(): Promise { try { await generate(codgegenConfig, true) diff --git a/client/web-sveltekit/src/lib/graphql/urql.ts b/client/web-sveltekit/src/lib/graphql/urql.ts index 5c29cf85ee7..a4fb7ecdaf1 100644 --- a/client/web-sveltekit/src/lib/graphql/urql.ts +++ b/client/web-sveltekit/src/lib/graphql/urql.ts @@ -122,7 +122,8 @@ interface OperationResultState +// This needs to be exported so that TS type inference can work in SvelteKit generated files. +export interface InfinityQueryStore extends Readable> { /** * Reruns the query with the next set of variables returned by {@link InfinityQueryArgs.nextVariables}. diff --git a/client/web-sveltekit/src/lib/wildcard/Button.svelte b/client/web-sveltekit/src/lib/wildcard/Button.svelte index 7f298093736..40bdbe8eaa3 100644 --- a/client/web-sveltekit/src/lib/wildcard/Button.svelte +++ b/client/web-sveltekit/src/lib/wildcard/Button.svelte @@ -6,12 +6,16 @@ import { type BUTTON_DISPLAY, type BUTTON_SIZES, type BUTTON_VARIANTS, getButtonClassName } from './Button' - interface $$Props extends HTMLButtonAttributes { + type $$Props = { variant?: typeof BUTTON_VARIANTS[number] size?: typeof BUTTON_SIZES[number] display?: typeof BUTTON_DISPLAY[number] outline?: boolean - } + // This is already allowed by HTMLButtonAttributes ([key: `data-${string}`]: any) + // but for some reason it's not recognized when using `svelte-check` and an error + // is thrown instead. + 'data-testid'?: string + } & HTMLButtonAttributes export let variant: $$Props['variant'] = 'primary' export let size: $$Props['size'] = undefined diff --git a/client/web-sveltekit/src/lib/wildcard/menu/Menu.svelte b/client/web-sveltekit/src/lib/wildcard/menu/Menu.svelte index 3664401dc18..4873272b3be 100644 --- a/client/web-sveltekit/src/lib/wildcard/menu/Menu.svelte +++ b/client/web-sveltekit/src/lib/wildcard/menu/Menu.svelte @@ -4,9 +4,7 @@ interface DropdownMenuContext { item: DropdownMenu['elements']['item'] - trigger: DropdownMenu['elements']['trigger'] separator: DropdownMenu['elements']['separator'] - open: DropdownMenu['states']['open'] builders: DropdownMenu['builders'] } @@ -34,7 +32,7 @@ } = createDropdownMenu({ open, }) - setContext({ item, trigger, separator, builders, $open }) + setContext({ item, separator, builders })