Skip to content

Extension deprecation: Disable extension system with enableLegacyExtensions flag

Warren Gifford requested to merge ps/disable-extension-system into main

Created by: philipp-spiess

Closes #39047 (closed)

This PR disables the extension system by making the extensionController nullable.

There are a few reasons for doing it this way right now:

  • To deprecate the extension system properly, we need to understand all impacted areas.
  • Having the extensionController nullable will help authors of new code to know that this APIs are going away.
  • We can test the experience with a completely disabled extension system.

Even though the PR is quite massive, I followed a few simple rules to create it so reviewing should be relatively easy:

  • There is only one place where we query the feature flag. Everything else is based on the extension controller being null.
  • Since the extension controller is passed through a lot of components using a shared interface, I started by making this interface nullable: https://github.com/sourcegraph/sourcegraph/pull/40247/files#diff-85457ce01cfd3f9507d02d870707300a6945081cfd4b8b9d8e119ad77f55c05eR47
    • I decided to make it null instead of optional to make all the checks more explicit (i.e instead of typeof x == 'undefined' we can do !== null)
  • In addition, there are a few components that are only used for extensions. For this I created a new interface RequiredExtensionsControllerProps that mimics the previews one where the extension controller can not be nullable.
  • All of the other changes are based on TypeScript errors and simply gate out any extension controller calls if it is null. For components that require the controller, we gate out the whole component from the tree.

This surely isn't perfect yet and there might be errors, but for now the important part is that this does not have any regressions when the extension controller is set (so we don't break prod).

Going forward, we need to do extensive testing to find out if any other product behavior is broken by the removal of the extension system beyond what we expect.

Code intel considerations

Since we do not have code navigation tools ported to a post-extension life, toggling enableLegacyExtensions will disable all code navigation features.

We're working on making this possible without the extension system but in case we need to ship this early, we can always just revert the one place where we depend on the feature flag and inject an extension controller that only exposes code navigation features.

Screenshot 2022-08-11 at 14 39 06 Screenshot 2022-08-11 at 14 36 11

Extension sidebar

I decided to keep the sidebar empty for now since we likely will need it for 4.0. One thing I have done was change the tooltip to call it "actions panel" instead of "extensions panel":

Bundle size wins

I made the extension system lazy loaded and measured the total transferred data in dev mode on a blob view page between when the flag is on vs. when it is off. Guess which screenshot is what flag state:

Screenshot 2022-08-11 at 15 34 04

Screenshot 2022-08-11 at 15 34 32

Test plan

As I mentioned above, my focus was an ensuring no regressions when an extension controller is present. Our automatic tests should help with this but I've also done manual testing, e.g.:

Screenshot 2022-08-11 at 14 39 06

App preview:

Check out the client app preview documentation to learn more.

Merge request reports

Loading