As a follow up to https://github.com/sourcegraph/sourcegraph/pull/61122 this commit updates the deprecated methods with the suggested replacements.
**Note**
This only migrates functions/methods that are replaced with something else, or whose deprecated call signature can easily be identified (e.g. `throwError(error)` -> `throwError(() => error)`). It's possible that there are more functions which deprecate a specific signature that we are using. I'll migrate those as I encounter them.
**Notes about `.toPromise`**
The instances of `.toPromise` converted here are all instances where the updated return value of `Promise<X|undefined>` did not produce a TS error (the ones with errors have been converted in #61122). However that doesn't mean that they can simply be replaced with `firstValueFrom`, `lastValueFrom` (these two methods throw errors when the source observable hasn't emitted a value before closing).
I update the callsites under two assumptions:
- Callsites that involve GraphQL requests will always emit a value and thus can be converted to using `lastValueFrom`/`firstValueFrom`.
- For other callsites we cannot make the assumption that the source observable emits before closing and thus they need a default value.
A side goal for the web rewrite is to leave the existing code base in a better state than before. I recently [added a hacky workaround](da5ddc99b6/client/web-sveltekit/vite.config.ts (L82-L101)) to make the Svelte version work properly with different rxjs versions. But the whole point of the rewrite is to not have to do these things anymore. So this is my attempt of upgrading rsjx in the main repo to v7.
I worked through the list of breaking changes in the [rxjs documentation](https://rxjs.dev/deprecations/breaking-changes) and fixed TypeScript issues to the best of my abilities.
Most notable changes:
- The custom `combineLatestOrDefault` operator was rewritten to avoid using rxjs internals, and the `.lift` method (side note: the corresponding tests do not cover all expected behavior, but issues had been caught through other tests)
- Where necessary `.toPromise()` was replaced with `lastValueFrom` or `firstValueFrom`. My assumption was that since we don't get runtime errors for the existing code, it's save to assume that the corresponding observables emit at least one value, i.e. `.toPromise()` did not return `undefined`. Only in some places I added a default value where it was easy to deduce what it should be.
- The generic type in `of()` was removed
- The generic type in `concat()` was removed
- `Subject.next` seemed to have allowed `undefined` to be passed even if the subject's types didn't allow that. If the subject's type couldn't be extended to include `undefined` I changed the code to not pass `undefined`.
- The generic type signature of `defaultIfEmpty` changed.
- Where possible I replaced `Subscribable` with `ObservableInput`, but we also have a copy of the old `Subscribable` interface in the `sourcegraph` package, and that makes things more complicated.
- I simplified unnecessary Promise/Observable interop where necessary.
A lot of the complex rxjs logic and usage of changed interfaces, such as `Subscribable`, is in extensions related code, which is not used in the web app anymore, but is still at least imported in the browser extensions code. Most of it is probably not used anymore, which makes the migration somewhat simpler.
* add initial aspect workflow yaml
- try reading docker config env var
- bump both timers
- bump grpc test timout to long
- skip additional perforce test and run all tests
- bump timeouts
- more timeout bumps and skip p4 test
- bump doc:test timeout
- bump e2e_test timeout
- bump database/connections/live timeout
- tag integration tests as exclusive
* add recommended bazelrc in workflows to speed up cold builds
* disable experimental_fetch_all_coverage_outputs
* port changes from https://github.com/sourcegraph/sourcegraph/compare/aspect-trial/wb-add-initial-config...aspect-trial/wb-add-initial-config-greg
* bazel configure
* add //:postcss_config_js as data target to client/web
* remove postcss added in debug
* use node-fetch and only test codeintellify
* use testing fetch.js setup
* fix syntax in testSetup
* various fixes
revert timeout bump on repository test
re-enable git p4 test
add testing from shared deps
bazel configure
* update comments on skipped tests
* restore `is_percy_enabled` for mocha_test
* slightly increase repo cloning wait
* use process.cwd instead of __dirname
* set sizing to moderate as well for embeddings
* remove setting CI in workflows yaml
* fix sizing
* workflow yaml tweaks and bazelrc tweaks
* make bazelrc consistent with what was in workflow yaml
---------
Co-authored-by: Jean-Hadrien Chabran <jh@chabran.fr>
Co-authored-by: Greg Magolan <greg@aspect.dev>
* reapply "switch from jest to vitest for faster, simpler tests (https://github.com/sourcegraph/sourcegraph/pull/57886)"
This was reverted in https://github.com/sourcegraph/sourcegraph/pull/58116 due to an issue with the browser tests.
* include fetch-mock
* fix flakiness
* rm mock that did not work in experimentalVmThreads
* fix
* timeout
* fixup
---------
Co-authored-by: William Bezuidenhout <william.bezuidenhout@sourcegraph.com>
When running vitests concurrently the target //client/browser:test fails
Revert "switch from jest to vitest for faster, simpler tests (#57886)"
This reverts commit ae5325e432.
Replaces our usage of jest with vitest. Also removes the babel transpiler. This simplifies our test configuration by a lot, makes tests run 10% faster, and makes further modernizations to our build/test stuff possible (such as using vite for frontend builds).
This removes some of the junit exporting for Buildkite, and the vitest bazel defs don't really cleanly implement bazel testing guidelines (like sharding). But vitest is only used for unit tests (all integration/e2e/regression tests have always run in mocha), so none of them are very slow anyway.
## Codemods for vitest imports
fastmod -e js,ts,tsx @jest/globals vitest client/ dev/release/
fastmod -e js,ts,tsx 'jest\.(\w+)\(' 'vi.$1(' client/ dev/release/
fastmod -e js,ts,tsx 'jest,' 'vi,' client/ dev/release/
fastmod -e js,ts,tsx 'jest }' 'vi }' client/ dev/release/
git diff --diff-filter=M --name-only | xargs pnpm exec prettier --write
See https://github.com/typescript-eslint/typescript-eslint/pull/6754.
Also removes needless `.eslintrc.js` files (now that we can use the root `tsconfig.all.json` for linting and it's still fast enough).
Some of our eslint rules were unintentionally made ineffective in `client/web`, and this commit also re-enables them and in some cases suppresses the eslint warning where a fix is not urgent.
Previously bindings like `expect`, `describe`, `afterAll`, etc., were imported implicitly by Jest or Mocha. We should import them explicitly to reduce magic. (Also this makes it easier to migrate to another test runner in the future if we want.)
* disable slow eslint rules, remove unused disable directives
Disabling these eslint rules makes saving significantly faster. These rules are not worthless, but they are usually ignored anyway, and I can't recall a specific instance when they would have caught a bug. I am proposing we disable them and then set a checkpoint in 14 days to rerun eslint with the rules enabled and see if they would have caught any bad practices. In the meantime, we will all benefit from instant saves (with eslint fixes) instead of waiting 3-5 seconds or more after each save in VS Code, which is destructive to productivity.
* upgrade eslint
Use [esbuild](https://esbuild.github.io/) instead of Webpack for builds of `client/web`, for faster builds (dev and prod) and greater dev-prod parity. This PR completely removes all use of Webpack in this repository.
`client/web` is the last build target that still uses Webpack; all others have been recently migrated to esbuild. Most devs here have been using esbuild for local dev of `client/web` for the last 6-12 months anyway. The change here is that now our production builds will be built by esbuild.
All sg commands, integration/e2e tests, etc., continue to work as-is. The bundlesize report will take a while to stabilize because the new build products use different filenames.
## Benchmarks
Running `pnpm run generate && time pnpm -C client/web run task:gulp webBuild` and taking the `time` output from the last command:
- Webpack: 62.5s
- esbuild: 6.7s
Note: This understates esbuild's victory for 2 reasons: (1) because esbuild is building both the main and embed entrypoints, whereas Webpack only builds the main entrypoint in this benchmark) and (2) because a lot of it is in the fixed startup time of `gulp`; esbuild incremental rebuilds during local dev only take ~1s.
## Notes
We no longer use Babel to produce web builds (we use esbuild), so we don't need any Babel plugins that optimize the output or improve browser compatibility. Right now, Babel is only used by Jest (for tests) and by Bazel as an intermediate step.
This PR adds the eslint rules @typescript-eslint/consistent-type-imports and @typescript-eslint/no-import-type-side-effects. Having type imports be explicitly declared as such makes it easier for modern bundlers to strip those imports which is useful if e.g. a module is sent directly to the browser.
I added both rules as a warning because @typescript-eslint/consistent-type-exports is also a warning.
Required adding a few import ignore directives in
`client/web/BUILD.bazel`. @jbedard is working on a fix for those. Has to
do with tsconfig paths. Fix will land in a future release.
## Test plan
5.4.11 output:
```
$ bazel configure
Updating BUILD files for protobuf, go, javascript
WARN[0000] Failed to load base tsconfig file @sourcegraph/tsconfig: open /Users/greg/aspect/sourcegraph/sourcegraph/@sourcegraph/tsconfig: no such file or directory
client/app-shell/src/app-shell.tsx parse error(s):
50: Top-level await is currently not supported with the "iife" output format
Failed to parse tsconfig file client/completions-review-tool/tsconfig.json: ERR: position:298 object key must be string pos:298 :
933 BUILD files visited
0 BUILD files updated
```
5.5.2 output has some new spam as well:
```
$ bazel configure
Updating BUILD files for protobuf, go, javascript
WARN[0000] Failed to load base tsconfig file @sourcegraph/tsconfig: open /Users/greg/aspect/sourcegraph/sourcegraph/@sourcegraph/tsconfig: no such file or directory
Failed to parse tsconfig file client/completions-review-tool/tsconfig.json: ERR: position:298 object key must be string pos:298 :
client/web/src/globals.d.ts:
17: }
^
client/web/src/storm/backend/route-loader.ts:
41: usePreloadedQueryData: () => ReturnType<typeof useSuspenseQuery<D, V>>
^
41: usePreloadedQueryData: () => ReturnType<typeof useSuspenseQuery<D, V>>
^
934 BUILD files visited
0 BUILD files updated
```
- Revert "revert "bazel: improve ESLint rule" (#52853)"
- bazel: fix eslint custom rule so js_binary runfiles are included as
tool inputs to ctx.actions.run_shell
## Test plan
Tested locally that fix commit resolves the
```
FATAL: aspect_rules_js[js_test]: RUNFILES environment variable is not set
```
flaky issue that prompted the revert
- Custom ESLint Bazel rule now relies on `sh_test`. The build part of
the rule produces the output file with ESLint errors, and the `sh_test`
target verifies that it's empty. If it's not empty, the ESLint test
fails, and the report content is printed to stdout.
- Added additional ESLint targets to `*.js` files in the root of each
client package.
- Added additional ESLint targets for `*.story.tsx` files for client
packages with stories. It's temporary until we start building Storybook
story modules with `ts_project`.
- Disabled ESLint outside of Bazel: **10-12m job is gone!** 🎉
## Test plan
bazel test `bazel query 'attr("name", ".*_eslint$", //client/...)'`
- Upgraded `aspect_bazel_lib`, `aspect_rules_js` and `aspect_rules_ts`
to the latest versions.
- Ran [bazel run
//.aspect/bazelrc:update_aspect_bazelrc_presets](40a7422385)
- 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/...)')`
Webpack bundles compile but need further testing. Jest + mocha tests
compile but are marked as `manual` until further work is done to get them
passing. The four jest tests are green and enabled now, though.
## Test plan
`bazel build //client/...` and `bazel test //client/...`
Bazel's rules_js rely on the pnpm package manager. To simplify the integration, we're migrating to pnpm from our current package manager — yarn. Another reason to migrate is that pnpm is cool and fast. 😉
We have deprecated the extension API (`sourcegraph` module) and want to remove it entirely. The `@sourcegraph/shared/src/codeintel/legacy-extensions/api` module already duplicates most of the `sourcegraph` exports and does not have the implicit representation of being an external API. This commit is one step along the way toward removing the `sourcegraph` module and package.
* chore: tables in code excerpts should use headers for line numbers
* chore: apply td to th for data-line in syntax-highlighter image
* feat: add more changes to fix blob view issues
* fix: ui review changes
* fix: percy snapshots
Problem: When selecting multiple lines such that the first selected line
is out of view, the file view jumps back to the first selected line
after selecting the last line.
This is because we are only considering the first selected line when we
determine the position to scroll to.
This commit changes the logic to take into account the top and bottom of
the complete line range.
This is the first, experimental, tech preview, proof-of-concept version of a better reference panel to get internal user feedback.
Use the feature flag `"experimentalFeatures": { "coolCodeIntel": true }` to try it.
Requirement: it only works with precise code intel data now.
This fixes https://github.com/sourcegraph/sourcegraph/issues/30969
