We have a number of docs links in the product that point to the old doc site.
Method:
- Search the repo for `docs.sourcegraph.com`
- Exclude the `doc/` dir, all test fixtures, and `CHANGELOG.md`
- For each, replace `docs.sourcegraph.com` with `sourcegraph.com/docs`
- Navigate to the resulting URL ensuring it's not a dead link, updating the URL if necessary
Many of the URLs updated are just comments, but since I'm doing a manual audit of each URL anyways, I felt it was worth it to update these while I was at it.
* 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.)
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/...)')`
Previously, the behavior was different whether you clicked on the "Go to
definition" link in the popover or used cmd-click. When using the link,
the current location didn't get pushed into the browser history making
"Go back" jump to a surprising location. When using cmd-click to
navigate to a definition, we pushed the current location into the
browser history to replicate the experience of navigating code in the
editor where you can easily go up/down the "jump stack", which is the
list of locations you navigate from/to with "Go to definition".
Importantly, this PR preserves the HTML link in the popover so that
users can still right-click "Open link in new tab".
## Test plan
Manually tested with
```
❯ SOURCEGRAPHDOTCOM_MODE=true SOURCEGRAPH_API_URL=https://sourcegraph.com sg start web-standalone
```
- Open URL
https://sourcegraph.test:3443/github.com/sourcegraph/sourcegraph/-/blob/cmd/gitserver/server/server.go?L1080
- Load popover for `serverContext` on line 1093
- Click "Go to definition"
- "Go back" in the browser history and verify that you go back to line
1093 (not 1080!)
- Load the popover again for `serverContext`
- Use cmd-click (or ctrl-click on Linux/Windows) on the "Go to
definition" *link* and verify that it opens the definition in a new
browser tab. Do the same for shift-click to open the link in a new
window.
- Load the popover again for `serverContext`
- Verify that you can right-click on the "Go to definition" *link* and
use "Open Link in New Tab" to open the definition location in a new
browser tab
Going back in history
<!-- All pull requests REQUIRE a test plan:
https://docs.sourcegraph.com/dev/background-information/testing_principles
-->
## App preview:
- [Web](https://sg-web-olafurpg-definition-recover.onrender.com/search)
Check out the [client app preview
documentation](https://docs.sourcegraph.com/dev/how-to/client_pr_previews)
to learn more.
---------
Co-authored-by: Taras Yemets <yemets.taras@gmail.com>
This PR removes old views extension logic that was used primary for the
first prototype version of code insights and it's no longer used
anywhere in our application at the moment.
## Test plan
- Make sure that CI is green and there is no regression in still used
extension logic
- Remove support for toggling native tooltips from the browser
extension. This let users choose to keep the browser extension installed
and partially active on GitHub but not show Sourcegraph's hovers. This
functionality is less important now that GitHub's new code nav no longer
uses hovers. For old GitHub Enterprise server instances (and GitHub.com
users who have not enabled the new code view), users can disable the
browser extension if they want to disable Sourcegraph's hover
functionality.
- Remove hover alerts, which were used to warn users that the results
are imprecise. We still show this in a hover badge, which is a much
nicer UI for this than a dismissible warning (which felt more "CYA").
- Remove command palette because it was inextricable from notifications
and it was only used by the Sourcegraph extension API (which is
deprecated and will be removed).
- Remove the old notifications UI, which showed notification messages in
Sourcegraph and code host UIs. This is no longer necessary with the
removal of the command palette, because it is no longer possible to
invoke long-running actions whose errors must be shown in a separate,
global UI.
## Test plan
Existing tests suffice to test the existing code intelligence
functionality. This PR just removes functionality.
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. 😉
Removes the extension registry UI and the GraphQL API that is necessary for the registry extension UI.
Any extensions that already exist locally or remotely can still be used with the enableLegacyExtensions site config flag and by enabling the extensions in your user settings.
This change is an intermediate step toward removing the extension registry (except the endpoint that's needed to support pre-4.0 Sourcegraph instances).
I think this PR has a 99% chance of going into the 4.4 release (2023-01-20) and not needing to be reverted. Subsequent PRs that actually make it impossible to use local or custom legacy extensions are still pending customer communication and confirmation. I hope we can merge other future PRs there as well, but there is a greater chance we'd need to revert them before 4.4. By making this PR first, it means less of a revert and more progress regardless of what we learn from customers.
* Implement token cursor using CodeMirror selections
Previously, the token cursor feature flag used HTML href links for go to
definition. This commit changes the behavior of the feature flag to use
CodeMirror selections instead along with several other big changes:
* Cmd+Click for Go to definition
* Enter for Go to definition
* Space for Show popover
* Right-click menu for Go to definition and Find references
* Up/Down/Right/Left arrow to move the selection to the next token
Demo https://www.loom.com/share/a0fa53badd4d419081dcfd1a2fbc7e4e
Please give it a try and let us know what you think.
* Fix failing integration tests
* Hide popover when moving the selection
* Close hover tooltip on escape
* Replace dom manipulation with idiomatic CodeMirror extension
* Revert ARIA role='main'
* Address review feedback
* s/occurence/occurrence
