Skip to content

Commit

Permalink
bazel: implement custom ESLint Bazel rule (sourcegraph#52062)
Browse files Browse the repository at this point in the history
- Upgraded `aspect_bazel_lib`, `aspect_rules_js` and `aspect_rules_ts`
to the latest versions.
- Ran [bazel run
//.aspect/bazelrc:update_aspect_bazelrc_presets](sourcegraph/sourcegraph@40a7422)
- Added `eslint_config` macro for client package eslint configuration
`js_library` targets.
- Implemented the custom ESLint rule, which copies `srcs` with
dependencies and **declarations** to the Bazel to lint them. This way,
we maintain the ability to do type-aware linting in Bazel.
- Added a custom ESLint formatter used in Bazel to print out relative
paths in ESLint reports.

In the follow-up PR, I will look into improvements suggested by
@alexeagle that should allow us to convert ESLint build targets into
test targets and gracefully manage linting failures.

## Test plan

1. CI
2. `bazel build $(bazel query 'kind("_eslint_test_with_types",
//client/...)')`
  • Loading branch information
valerybugakov authored May 22, 2023
1 parent 2d2538f commit 760db94
Show file tree
Hide file tree
Showing 54 changed files with 638 additions and 78 deletions.
10 changes: 6 additions & 4 deletions .aspect/bazelrc/ci.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
# or split up into multiple test targets with sharding or manually.
# Set this flag to exclude targets that have their timeout set to eternal (>15m) from running on CI.
# Docs: https://bazel.build/docs/user-manual#test-timeout-filters
build --test_timeout_filters=-eternal
test --test_timeout_filters=-eternal

# Set this flag to enable re-tries of failed tests on CI.
# When any test target fails, try one or more times. This applies regardless of whether the "flaky"
Expand All @@ -16,7 +16,7 @@ build --test_timeout_filters=-eternal
# is more likely to get fixed.
# Note that when passing after the first attempt, Bazel will give a special "FLAKY" status.
# Docs: https://bazel.build/docs/user-manual#flaky-test-attempts
build --flaky_test_attempts=2
test --flaky_test_attempts=2

# Announce all announces command options read from the bazelrc file(s) when starting up at the
# beginning of each Bazel invocation. This is very useful on CI to be able to inspect what Bazel rc
Expand All @@ -29,9 +29,11 @@ build --announce_rc
# Docs: https://bazel.build/docs/user-manual#show-timestamps
build --show_timestamps

# Only show progress every 5 seconds on CI.
# Only show progress every 60 seconds on CI.
# We want to find a compromise between printing often enough to show that the build isn't stuck,
# but not so often that we produce a long log file that requires a lot of scrolling.
# https://bazel.build/reference/command-line-reference#flag--show_progress_rate_limit
build --show_progress_rate_limit=5
build --show_progress_rate_limit=60

# Use cursor controls in screen output.
# Docs: https://bazel.build/docs/user-manual#curses
Expand Down
14 changes: 0 additions & 14 deletions .aspect/bazelrc/performance.bazelrc
Original file line number Diff line number Diff line change
@@ -1,17 +1,3 @@
# Merkle tree calculations will be memoized to improve the remote cache hit checking speed. The
# memory foot print of the cache is controlled by `--experimental_remote_merkle_tree_cache_size`.
# Docs: https://bazel.build/reference/command-line-reference#flag--experimental_remote_merkle_tree_cache
build --experimental_remote_merkle_tree_cache
query --experimental_remote_merkle_tree_cache

# The number of Merkle trees to memoize to improve the remote cache hit checking speed. Even though
# the cache is automatically pruned according to Java's handling of soft references, out-of-memory
# errors can occur if set too high. If set to 0 the cache size is unlimited. Optimal value varies
# depending on project's size.
# Docs: https://bazel.build/reference/command-line-reference#flag--experimental_remote_merkle_tree_cache_size
build --experimental_remote_merkle_tree_cache_size=1000
query --experimental_remote_merkle_tree_cache_size=1000

# Speed up all builds by not checking if output files have been modified. Lets you make changes to
# the output tree without triggering a build for local debugging. For example, you can modify
# [rules_js](https://github.com/aspect-build/rules_js) 3rd party npm packages in the output tree
Expand Down
6 changes: 6 additions & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
**/graphql-operations.ts
**/node_modules/**
out/
dist/
src/schema/*
src/graphql-operations.ts
GH2SG.bookmarklet.js
Expand All @@ -7,3 +10,6 @@ svelte.config.js
vite.config.ts
playwright.config.ts
.vscode-test
**/*.json
**/*.d.ts
eslint-relative-formatter.js
4 changes: 4 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,10 @@ See https://handbook.sourcegraph.com/community/faq#is-all-of-sourcegraph-open-so
'never',
{
schema: 'always',
scss: 'always',
css: 'always',
yaml: 'always',
svg: 'always',
},
],
'import/order': 'off',
Expand Down
63 changes: 63 additions & 0 deletions BUILD.bazel

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

38 changes: 29 additions & 9 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,23 @@ bazel_skylib_workspace()

http_archive(
name = "aspect_bazel_lib",
sha256 = "2518c757715d4f5fc7cc7e0a68742dd1155eaafc78fb9196b8a18e13a738cea2",
strip_prefix = "bazel-lib-1.28.0",
url = "https://github.com/aspect-build/bazel-lib/releases/download/v1.28.0/bazel-lib-v1.28.0.tar.gz",
sha256 = "0da75299c5a52737b2ac39458398b3f256e41a1a6748e5457ceb3a6225269485",
strip_prefix = "bazel-lib-1.31.2",
url = "https://github.com/aspect-build/bazel-lib/releases/download/v1.31.2/bazel-lib-v1.31.2.tar.gz",
)

http_archive(
name = "aspect_rules_js",
sha256 = "3e237129b3554373a80c681c4b47348f91c294ff32d4bc8f8297f40511a4eb6c",
strip_prefix = "rules_js-1.25.4",
url = "https://github.com/aspect-build/rules_js/releases/download/v1.25.4/rules_js-v1.25.4.tar.gz",
sha256 = "08061ba5e5e7f4b1074538323576dac819f9337a0c7d75aee43afc8ae7cb6e18",
strip_prefix = "rules_js-1.26.1",
url = "https://github.com/aspect-build/rules_js/releases/download/v1.26.1/rules_js-v1.26.1.tar.gz",
)

http_archive(
name = "aspect_rules_ts",
sha256 = "02480b6a1cd12516edf364e678412e9da10445fe3f1070c014ac75e922c969ea",
strip_prefix = "rules_ts-1.3.1",
url = "https://github.com/aspect-build/rules_ts/releases/download/v1.3.1/rules_ts-v1.3.1.tar.gz",
sha256 = "ace5b609603d9b5b875d56c9c07182357c4ee495030f40dcefb10d443ba8c208",
strip_prefix = "rules_ts-1.4.0",
url = "https://github.com/aspect-build/rules_ts/releases/download/v1.4.0/rules_ts-v1.4.0.tar.gz",
)

http_archive(
Expand Down Expand Up @@ -95,6 +95,26 @@ npm_translate_lock(
npm_package_target_name = "{dirname}_pkg",
npmrc = "//:.npmrc",
pnpm_lock = "//:pnpm-lock.yaml",
# Required for ESLint test targets.
# See https://github.com/aspect-build/rules_js/issues/239
# See `public-hoist-pattern[]=*eslint*` in the `.npmrc` of this monorepo.
public_hoist_packages = {
"@typescript-eslint/eslint-plugin": [""],
"@typescript-eslint/[email protected]_qxbo2xm47qt6fxnlmgbosp4hva": [""],
"eslint-config-prettier": [""],
"eslint-plugin-ban": [""],
"eslint-plugin-etc": [""],
"eslint-plugin-import": [""],
"eslint-plugin-jest-dom": [""],
"eslint-plugin-jsdoc": [""],
"eslint-plugin-jsx-a11y": [""],
"[email protected]_eslint_8.34.0": [""],
"eslint-plugin-react-hooks": [""],
"eslint-plugin-rxjs": [""],
"eslint-plugin-unicorn": [""],
"eslint-plugin-unused-imports": [""],
"eslint-import-resolver-node": [""],
},
verify_node_modules_ignored = "//:.bazelignore",
)

Expand Down
1 change: 1 addition & 0 deletions client/app-shell/.eslintignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
dist/
1 change: 0 additions & 1 deletion client/app-shell/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ module.exports = {
{
files: 'dev/**/*.ts',
rules: {
'no-console': 'off',
'no-sync': 'off',
},
},
Expand Down
8 changes: 7 additions & 1 deletion client/app-shell/BUILD.bazel

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 7 additions & 5 deletions client/app-shell/src/app-shell.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { listen, Event } from '@tauri-apps/api/event'
import { invoke } from '@tauri-apps/api/tauri'

import { logger } from '@sourcegraph/common'

// Sourcegraph desktop app entrypoint. There are two:
//
// * app-shell.tsx: before the Go backend has started, this is served. If the Go backend crashes,
Expand All @@ -21,7 +23,7 @@ async function getLaunchPathFromTauri(): Promise<string> {
async function launchWithSignInUrl(url: string): Promise<void> {
const launchPath = await getLaunchPathFromTauri()
if (launchPath) {
console.log('Using launch path:', launchPath)
logger.log('Using launch path:', launchPath)
url = addRedirectParamToSignInUrl(url, launchPath)
}
window.location.href = url
Expand All @@ -35,16 +37,16 @@ const appShellReady = (payload: AppShellReadyPayload): void => {
if (!payload) {
return
}
console.log('app-shell-ready', payload)
logger.log('app-shell-ready', payload)
launchWithSignInUrl(payload.sign_in_url).catch(error =>
console.error(`failed to launch with sign-in URL: ${error}`)
)
}

listen('app-shell-ready', (event: Event<AppShellReadyPayload>) => appShellReady(event.payload))
.then(() => console.log('registered app-shell-ready handler'))
.catch(error => console.error(`failed to register app-shell-ready handler: ${error}`))
.then(() => logger.log('registered app-shell-ready handler'))
.catch(error => logger.error(`failed to register app-shell-ready handler: ${error}`))

await invoke('app_shell_loaded')
.then(payload => appShellReady(payload as AppShellReadyPayload))
.catch(error => console.error(`failed to inform Tauri app_shell_loaded: ${error}`))
.catch(error => logger.error(`failed to inform Tauri app_shell_loaded: ${error}`))
3 changes: 3 additions & 0 deletions client/branded/BUILD.bazel

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions client/browser/BUILD.bazel

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions client/build-config/BUILD.bazel

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions client/client-api/BUILD.bazel

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions client/codeintellify/BUILD.bazel

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions client/cody-cli/BUILD.bazel

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions client/cody-shared/.eslintignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
/dist/
/out/
Loading

0 comments on commit 760db94

Please sign in to comment.