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.
* Improve testing for dev instances
S2 sets the `dev` tag, so checking UserCount helps confirm when a dev license is used
* Use current license public key to identify dev instances
* Add comment to publicKeyData
* Update comparison
* Tweak comment
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.)
Our tracking of frontend (TypeScript) code coverage has decayed and was no longer being used. The Codecov report at https://app.codecov.io/gh/sourcegraph/sourcegraph significantly under-counts coverage, and what coverage tasks we did have were flaky in CI and relied on old libraries that have not been updated recently to keep with modern practices (such as `@storybook/addon-storyshots` only supporting Jest with `injectGlobals`).
From https://sourcegraph.slack.com/archives/C04MYFW01NV/p1698059032863009?thread_ts=1698054092.709309&cid=C04MYFW01NV, the proposal is to remove coverage tracking for now and prioritize getting tests to run really fast and smooth locally first, including upgrading our testing infra, and then reenable code coverage tracking.
* 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
I wanted to look into this resolver because it seemed to cause a lot of load on S2 and dotcom.
Looking at it, I realize that there are zero tests so I didn't even know what's expected to be returned here.
This PR adds a baseline for what should be returned and fixed a bunch of bugs:
- Improper argument validation that could cause panics
- Improperly counted results for `first` argument
- Removed arguments from GraphQL schema that weren't actually respected
- Fix line ending handling in content pagination to return indication if file ended with no newline
- Fix TotalLines for files without a final newline
- Improves performance of line parsing
- Fix ancestors option creating too many results by recursively including all children into the set again (was able to get 90000 entries returned from S2 for a relatively small folder structure)
- Make behavior of recursiveSingleChild match what's documented in GraphQL. Before we would only recurse if the looked at entry is a single child (or first:1 was set)
Ultimately, the dir traversal we do in here should IMO happen in gitserver directly where we already have it in tree structure, but that will have to be for later.
For now, this mostly adds some confidence to these resolvers.
Attempting to understand when we schedule repo updates, I found a lot of places where we didn't explicitly say that we don't need to ensure the revision, but based on assumptions about data above we can safely do so.
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.
Update install-vscode.md
To:
- fix typo 'sugsgestions'
- add missing word 'extension' to 'set up the Cody within your VS Code environment'
- fix indentation
Co-authored-by: Justin Dorfman <justin.dorfman@sourcegraph.com>
Fixes `tsc --build` errors of the form:
```
error TS2742: The inferred type of 'default' cannot be named without a reference to '.pnpm/@storybook+types@7.4.6/node_modules/@storybook/types'. This is likely not portable. A type annotation is necessary.
```
With this rule enabled, saving a file in VS Code often takes 2-5 seconds or longer. With it disabled, saving a file is usually instant.
This rule is identified as the biggest culprit in slow eslint analysis by running `TIMING=1 pnpm exec eslint --fix client/web/dev/server/development.server.ts`.
To pass multiple ignore patterns to Jest, you need to use `--testPathIgnorePatterns pattern1 --testPathIgnorePatterns pattern2 ...`. Otherwise, `--testPathIgnorePatterns pattern1 pattern2` treats `pattern2` as a test pattern to run, not to ignore.
The consequence of this is that some npm scripts and CI steps were running tests that they didn't need to run.
The “v1” query input remains the default. But now the code and settings refer to the new (formerly “experimental”) query input as “v2”. If `experimentalFeatures.searchQueryInput` is set to `experimental`, it will behave as though it is set to `v2`.
* remove redaction from cookie urls
removes redaction from url cookies as we no longer host private code on sourcegraph.com
* Update eventLogger.ts
* Update eventLogger.ts
* Update eventLogger.ts
* Update eventLogger.ts
* expand the logic to all marketing fields
* add additional subdomain regex filtering since isSourcegraphDotComMode is flagged on customer instance's too
* if the site_id is anything other than sourcegraphWeb, have all urls go through redaction
* update tests to test logic between cloud instances and dotcom
* add list of approved hosts where we do not redact from
* sg lint
* update redaction to only happen in one place, send redacted urls to BQ for extra measure and add tests
* sg lint
* add more test cases + re-add URL redaction on cookie side
* bazel fix
* revert referrer logic since referrer is not captured on events_usage
* updated logic to handle referrers on cloud instances + redact session_first_url as a safe measure
* add redaction to sesion_referrer and to originial_referrer to future-proof against ELE
* Migrate URL redaction to backend
refactor: Remove frontend URL redaction
Removed the frontend URL redaction logic in eventLogger.ts for firstSourceURL, lastSourceURL, originalReferrer, sessionReferrer, and sessionFirstURL. This redaction will now be handled in the backend.
Removed calls to redactSensitiveInfoFromAppURL and related cookie logic in eventLogger.ts. URL redaction is now consolidated in the backend.
* Update event_handlers_test.go
Update redaction test for referrer logic. Should only apply to managed instances since we have this clause in the function:
```
if envvar.SourcegraphDotComMode() {
return rawURL, nil
}
* fix ineffectual assignment to err
* remove `redactSensitiveInfoFromAppURL` + update dotComModeVariable
since migrating redaction to backend, can remove this function + update ref to dotComVariable
* the `redactSensitiveInfoFromCloudURL` now only takes the first part of the path and redacts all query parameters except UTM
* undo redaction for referrer values + add comment on redaction
* Apply suggestions from code review
Co-authored-by: Dan Adler <dadlerj@users.noreply.github.com>
* update url path redaction logic and associated tests
* read cookie value only when its confirmed to be dotcom mode (optimization)
---------
Co-authored-by: Dan Adler <dadlerj@users.noreply.github.com>
In an attempt to simplify the set of ways a repo might be scheduled for an update, I found this endpoint and believe it isn't used and that it shouldn't be needed. We have all this in the UI as well, no need to expose a separate HTTP endpoint that doesn't even seem to have admin restrictions.
This aligns what we do for Go with what we do for python, writing a zip file to disk.
This should prevent big memory spikes when large go modules are downloaded.
Combine #57751 and #57756 to fork p4-fusion into a separate package, and update relevant images.
We've forked the tool and are now ready to try some of the fixes out in a more production enviroment, so bundling this temporarily from a specific commit from the fork repo.
Next up, I'll open a PR to use the newly generated base image in bazel and add the new --noBaseCommit flag to the perforce syncer.
---------
Co-authored-by: Erik Seliger <erikseliger@me.com>
It turned out that we have two different ways of overriding feature
flags:
- `?feat=foo` overrides the feature flag on the server for this request
- `?feature-flag-key=foo&feature-flag-value=true` overrides the feature
flag on the client side only and persists the setting in local storage
This commit does three things:
1. It consolidates the two systems. `feature-flag-{key,value}` have been
removed in favor of `feat`. The client side logic will now use the
same parameter. An additional prefix has been added, `~` to indicate
the client to remove the override.
2. The header logic for GraphQL queries has been reworked to include
headers for overridden feature flags. This ensures that the server
uses the same overrides as the client.
3. The developer settings dialog and navbar entry now feature a reload
button which appends `flag` to the current URL for overridden feature
flags and reloads the page. Additionally the keyboard shortcut for
reloading the page (`Mod-r`) is hijacked to perform the same action.
NOTE: Unlike the first two, this functionality is only available in
development builds.
Implement RFC 837 https://docs.google.com/document/d/1aC4gHB8Q5lurwVhc8SCxznR0blNJMKo7yIkpP7WShno/edit.
This change will update our access token format to `sgp_<instance_identifier>_<40_char_access_token>`, allowing access tokens to be securely tied back to instances
This will initially be disabled behind a feature flag until Cody extension updates have been rolled out to users.
When running `sg test <target> extra-arg1 extra-arg2 ...`, the extra args are generally appended to the `<target>` command on a new line, which means the extra args are interpreted by the shell as their own command. The user's intent is for them to be appended to the last command in the `<target>` command string, which this commit achieves by trimming trailing whitespace.
