Part of https://github.com/sourcegraph/sourcegraph/issues/62448
Linear issue
[SRCH-573](https://linear.app/sourcegraph/issue/SRCH-573/integrate-cody-web-package-into-the-sourcegraph-ui)
This is highly experimental usage of the new (not currently merged but
published in NPM `cody-web-experimental`) package
## How to run it
- (Optional) if you previously linked any local packages make sure they
don't exist in your node_modules anymore, `rm -rf node_modules` in the
root then `pnpm install`
- Run standard `sg start web-standalone`
- Turn on `newCodyWeb: true` in your `experimentalFeatures`
## How to run it locally with prototype PR in Cody repository
- Open Cody repository on the `vk/integrate-cody-web-chat-2` branch
- At the root of the repo, run `pnpm install` to make sure you're up to
date with all of the dependencies.
- Go to the web package (`cd web`)
- Build it with `pnpm build`
- Create a global link with `pnpm link --global` (Ignore the warning
message about no binary)
- Open sourcegraph/sourcegraph repository on this PR branch
- Make sure you are in the root of the repo.
- Run `pnpm link --global cody-web-experimental`
- Run `sg start web-standalone` to bundle the web app and launch an
instance that uses S2 for the backend. You'll need to create a login on
S2 that is not federated by GitHub.
- Turn on `newCodyWeb: true` in your `experimentalFeatures`
- Have fun experimenting!
## Test plan
- Check that old version of Cody has got no regressions
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.
Fixes#60492
Completions and validation are not working in the settings editor. This is a huge problem because it (a) makes settings much more difficult to use (users don't know which settings exists or what they do) and (b) can lead to invalid settings configurations.
Co-authored-by: jamesmcnamara <james.mcnamara@sourcegraph.com>
* remove little-used `web-standalone-http-prod`
This let you run a local web app built in production mode against a remote Sourcegraph endpoint. You can still run a local web app built in *dev* mode.
* add `sg test bazel-backend-integration`
* fix DeveloperDialog positioning
It was taking up 100% width and was translated -50% so the left half of it was off-viewport.
* use vite for web builds
[Vite](https://vitejs.dev/) is a fast web bundler. With Vite, a local dev server is available as fast as with esbuild, and incremental builds are (1) hot (no page reload needed) and (2) much faster (<500ms).
* fix "manifestPlugin.d.ts" was not created
* sg lint
* remove little-used `web-standalone-http-prod`
This let you run a local web app built in production mode against a remote Sourcegraph endpoint. You can still run a local web app built in *dev* mode.
* add `sg test bazel-backend-integration`
* fix DeveloperDialog positioning
It was taking up 100% width and was translated -50% so the left half of it was off-viewport.
* use vite for web builds
[Vite](https://vitejs.dev/) is a fast web bundler. With Vite, a local dev server is available as fast as with esbuild, and incremental builds are (1) hot (no page reload needed) and (2) much faster (<500ms).
* fix "manifestPlugin.d.ts" was not created
* sg lint
* small lint fix
* added events shim to client/web/BUILD.bazel
* updated via bazel configure
* added in side-effectful import for EventEmitter
* added in side-effectful import for EventEmitter
* ran bazel configure
* re-run bazel configure
* pnpm dedupe
---------
Co-authored-by: William Bezuidenhout <william.bezuidenhout@sourcegraph.com>
Co-authored-by: jamesmcnamara <jamesscottmcnamara@gmail.com>
Co-authored-by: Jean-Hadrien Chabran <jh@chabran.fr>
* 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
swc is a very fast TypeScript transpiler.
Instead of using Babel for TypeScript transpilation in Bazel, we now use swc, which is much faster. We still use Babel in Jest (for tests), but in the future we intend to move away from Jest.
See https://docs.aspect.build/rulesets/aspect_rules_ts/docs/transpiler#swc-recommended for more information about using swc in Bazel, and https://swc.rs/ for general information about swc.
* stub method to avoid unhandled exception in test from pretendRemote
* less flaky ActionItem test that checks condition before snapshotting
* rename *.{spec => test}.ts
This simplifies the test filename pattern.
* rename top-level tsconfig.all.json to tsconfig.json
* upgrade to pnpm 8.9.2
* avoid usage of jsdom.reconfigure
* more robust linkClickHandler.test.tsx
* make getBundleSizeStats.test.ts not need to use mocks
* extract createBarrier() to @sourcegraph/testing
* more robust fromObservableQuery.test.ts
* avoid jsdom.reconfigure
* do not attempt to request assets from assets.gitlab-static.net
fastmod -F assets.gitlab-static.net example.com client/browser/src/
* avoid "incorrect casing" for mocked React components
* use createBarrier
* fix SurveyToast mock GraphQL query
* fix classNames typo
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.
* 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 removes another source of "names" and also makes it so that client/web can be built by esbuild via Bazel in the future (which does not support renaming entrypoints).
There are still many places that build the old non-enterprise bundle. This is no longer needed or maintained. This change simplifies our builds by removing that unnecessary target. There are still some remnants (such as `ui/assets/{enterprise,oss}`); not *all* are removed here.
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.
- 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/...)'`
We recently changed how assets are resolved in the backend. If the
backend is built with Bazel it just works, as well as when you run in
Dev mode.
But if the backend is built the normal go way, the assets are not where
the backend expects it to be. This fixes the frontend code to resolve
the assets directory depending on the environment
## Test plan
Check assets gets rendered with the following commands
* `sg start`
* `bazel build //enterprise/cmd/sourcegraph:sourcegraph`
* `./enterprise/dev/app/build-release.sh`
<!-- All pull requests REQUIRE a test plan:
https://docs.sourcegraph.com/dev/background-information/testing_principles
-->
- 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/...)')`
Use volatile status variables provided by stamping to inject relevant
Git information into the Percy execution process. Percy needs Git commit
and branch to create an accurate report and auto-accept changes on main.
See extensive comments in the code for implementation details.
On the high level, instead of running the Percy CLI directly from Bazel,
we wrap it into the JS script that:
1. Reads volatile status variables with Git info.
2. Creates a Percy CLI command with these variables injected.
3. Exectes the command using args received from the Bazel target.
## Test plan
1. CI
2. Locally: `bazel test //client/web/src/integration:integration-tests
--define=E2E_HEADLESS=true
--define=E2E_SOURCEGRAPH_BASE_URL="http://localhost:7080"
--define=GH_TOKEN=fake --define=XXX` — replace `XXX` with the
`PERCY_TOKEN` from our shared 1Password vault.
Adds client integration tests target, got it running locally and
disabled it for CI until the $DISPLAY issue is resolved.
## Test plan
1. `bazel test //client/web/src/integration:integration-tests`
2. [The CI build with Bazel
checks](https://buildkite.com/sourcegraph/sourcegraph/builds/210860) ✅
Actually, webpack_bundle doesn't have JS_BINARY__TARGET set since it is
a custom build rule.... To handle all the cases I think we have to check
for both. 😞
`BAZEL_BINDIR` is set for build targets only while `JS_BINARY__TARGET`
should be set for all js_binary & js_test targets for bazel build, test
& run contexts.
The `require.resolve` and `/index.js` are required for the provided
modules to resolve correctly under bazel, the added `dependencies` are
required because in the root they are `devDependencies` but
`build-config` needs them at runtime (note that "runtime" for
`build-config` is when webpack is run on `client/web`).
Share the way to test the web application enterprise build with Bazel.
## Test plan
1. `bazel build //client/web:bundle-enterprise`
2. To start the node.js server with the Bazel web application build.
`STATIC_ASSETS_PATH=XXX/sourcegraph/bazel-bin/client/web/bundle-enterprise
WEBPACK_SERVE_INDEX=true
SOURCEGRAPH_API_URL=https://sourcegraph.sourcegraph.com pnpm --filter
@sourcegraph/web run serve:prod`
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/...`
Other packages within `client/*` depend on `@sourcegraph/build-config`
as a dev dependency, but `@sourcegraph/build-config` itself depends on
these as runtime dependencies. Under bazel this dev vs non-dev is
stricter and `build-config` needs to declare the dependencies correctly.
Some of the deps are still in the root package.json as dev-deps when
they are still used elsewhere, others were removed from the root and are
now only in `build-config`.
The DEV_WEB_BUILDER_OMIT_SLOW_DEPS env var is an experimental dev env var for faster builds that I sometimes use. When using it, the packageResolution plugin needs the same fix as the one for stylePlugin in https://github.com/sourcegraph/sourcegraph/pull/46452 to work with pnpm. This is a generally sensible fix that implements the correct and desirable behavior, not just a way to make a hack work.
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. 😉
With the env var `DEV_WEB_BUILDER_OMIT_SLOW_DEPS` set (such as in `sg.config.overwrite.yaml`), esbuild rebuilds are ~40% faster. If you try to access pages that require these deps, you'll see a placeholder. This env var only takes effect when using esbuild (https://docs.sourcegraph.com/dev/background-information/web/build#esbuild), not Webpack.
