Loading unpacked extensions #1508

lguychard · 2018-12-19T18:43:32Z

closes #489

This PR adds the ability to load locally served Sourcegraph extensions. Review by commit.

The URL to an unpacked extension can be set using the debug palette:

In the browser extension, the unpacked extension url is stored in chrome.storage.local. This is done to avoid reading & fetching a URL from localStorage that may have been set by other scripts.

In the webapp, the value is read from localStorage.

By using localStorage/chrome.storage.local rather than settings, and only allowing one unpacked extension, we avoid usage of this feature to circumvent the "private extensions registry is a premium feature" restriction.

TODO:

styling
tests

This exposes the package.json when running `yarn serve`, which allows [loading unpacked extensions](sourcegraph/sourcegraph#1508)

sqs

High-level idea lgtm. Haven’t reviewed code.

codecov-io · 2019-01-02T18:11:13Z

Codecov Report

Merging #1508 into master will increase coverage by 0.02%.
The diff coverage is 87.5%.

Impacted Files	Coverage Δ
shared/src/api/client/services.ts	`100% <ø> (ø)`	⬆️
shared/src/api/integration-test/testHelpers.ts	`91.42% <100%> (ø)`	⬆️
shared/src/api/client/services.test.ts	`62.5% <100%> (ø)`	⬆️
.../src/api/client/services/extensionsService.test.ts	`88.23% <86.66%> (-2.68%)`	⬇️
...hared/src/api/client/services/extensionsService.ts	`69.73% <86.95%> (+4.73%)`	⬆️

lguychard · 2019-01-02T18:21:30Z

This is now ready for review

client/browser/src/browser/ExtensionStorageSubject.ts

shared/src/api/client/services/extensionsService.test.ts

ijsnow · 2019-01-02T18:24:57Z

shared/src/api/client/services/extensionsService.test.ts

-                            )
+                            ),
+                        cold('-a--|', { a: '' }),
+                        () => of(null)
                    ).activeExtensions
                )
            ).toBe('-a-b-|', {


Why is there an expected break in time? (the - between a and b)

shared/src/api/client/services/extensionsService.ts

sqs

High-level looks great. I added several comments about docs, impl, etc.

Also, you should add docs to doc/extensions/authoring/publishing.md about this new sideloading feature. Here is what I would recommend (feel free to add/change, just wanted to help out): documentation diff with TODOs. (One other awesome thing about this new feature is that it lets us remove a lot of complexity around the WIP extension creation/publishing process.)

shared/src/platform/context.ts

shared/src/extensions/controller.ts

shared/src/extensions/ExtensionStatus.tsx

sqs · 2019-01-03T04:22:08Z

shared/src/extensions/ExtensionStatus.tsx

+                            type="text"
+                            id="extension-status__unpackedurl"
+                            className="form-control"
+                            onChange={this.onUnpackedURLChange}


Will using onChange instead of waiting for blur or submit cause a lot of weird error messages if you are typing the URL in character-by-character? I think so, and that will probably confuse people. My guess is that the amount of effort now on your part required to avoid that confusion is worth it. (You could use a window.prompt(...) to avoid complicating the code here to handle the not-yet-committed state.)

Went with window.prompt. Here's the new experience:

sqs · 2019-01-03T04:23:19Z

shared/src/extensions/ExtensionStatus.tsx

+                            id="extension-status__unpackedurl"
+                            className="form-control"
+                            onChange={this.onUnpackedURLChange}
+                            placeholder="URL"


Suggested change

placeholder="URL"

placeholder="URL to extension's package.json"

And then modify the fetchUnpackedExtension code above slightly to treat the user's input URL as the package.json URL instead of appending as in ${baseUrl}/package.json.

This resolves the user's confusion "What URL do I put in there?" and makes it clear they need to make the package.json HTTP-accessible too (not just the extension JS file).

In the end, I decided against making this the URL to the package.json, and made it explicit that the user should enter the URL to the Parcel dev server. I also made this explicit in the publishing docs.

The extension creator will be updated to make sure the package.json is HTTP-accessible when running npm run serve.

Since parcel can only serve from dist/, our code assumes that the package.json is served from inside of dist/ (through a symlink, as set up by the extension creator), and that the relative path to the bundle should be fixed accordingly. I'd rather hide that complexity from the user.

shared/src/api/client/services/extensionsService.test.ts

shared/src/api/client/services/extensionsService.ts

client/browser/src/browser/ExtensionStorageSubject.ts

shared/src/platform/context.ts

Excluding WIP extensions from result sets added a lot of complexity to the code (API parameters) and the UI. It is sufficient to just sort them last. There will be fewer WIP extensions present anyway after #1508 is merged.

* remove extension title, use extension ID instead The extension title (the package.json extension manifest's `title` property) was redundant and added complexity: - It made it difficult to see the publisher of an extension on the extension registry, which is important for finding the right or trusted extension to use. - It was fairly redundant with the extension ID (`my-extension` vs `My extension`). - The presence of a title meant that the descriptions were longer and had boilerplate content (such as "This is a Sourcegraph extension that..."). Now that there is no title and it won't feel duplicative, descriptions can be shorter and more to-the-point. This, in turn, means that we can show the description on a single line in the extension registry list page, which reduces visual variance among extensions (some of which had multi-line descriptions, some of which had one-line descriptions). The only feature that required the use of the title was marking an extension as WIP. You can now do that by setting the extension manifest `wip` property to true. For backcompat, titles with `WIP:` and `[WIP]` prefixes still indicate WIP extensions. * show "Categories" dropdown next to extension registry search query input This lets users easily filter extensions by category. * add "Include WIP extensions" option to extension registry list page This removes the GraphQL includeWIP query parameter because it is no longer necessary. API clients can now express the same thing by including `#wip` in the query. It was only ever used by the immediate web app, never any other clients, so it is safe to remove. * add "Show enabled/disabled extensions" to the extension registry list page This lets users see only enabled extensions, or only disabled extensions. * remove include/exclude WIP, only sort WIP last Excluding WIP extensions from result sets added a lot of complexity to the code (API parameters) and the UI. It is sufficient to just sort them last. There will be fewer WIP extensions present anyway after #1508 is merged. * make category:"Other" also match extensions w/manifest with no category

ExtensionStorageSubject is an RxJS subject backed by an extension storage item

Co-Authored-By: lguychard <l.guychard@gmail.com>

lguychard requested review from chrismwendt, felixfbecker and ijsnow as code owners December 19, 2018 18:43

lguychard requested a review from sqs December 19, 2018 18:51

lguychard force-pushed the unpacked-extensions branch from 5ec2da1 to 8169f13 Compare December 19, 2018 19:08

lguychard mentioned this pull request Dec 19, 2018

Symlink package.json to dist/package.json when creating an extension sourcegraph/create-extension#39

Closed

sqs approved these changes Dec 20, 2018

View reviewed changes

lguychard force-pushed the unpacked-extensions branch from 8169f13 to 670bb31 Compare January 2, 2019 12:30

lguychard changed the title ~~WIP Loading unpacked extensions~~ Loading unpacked extensions Jan 2, 2019

felixfbecker reviewed Jan 2, 2019

View reviewed changes

client/browser/src/browser/ExtensionStorageSubject.ts Outdated Show resolved Hide resolved

ijsnow reviewed Jan 2, 2019

View reviewed changes

felixfbecker reviewed Jan 2, 2019

View reviewed changes

shared/src/api/client/services/extensionsService.ts Outdated Show resolved Hide resolved

sqs reviewed Jan 3, 2019

View reviewed changes

sqs mentioned this pull request Jan 3, 2019

improve extension registry list UI #1613

Merged

lguychard mentioned this pull request Jan 3, 2019

feat: symlink dist/package.json -> package.json when running serve sourcegraph/create-extension#41

Merged

lguychard force-pushed the unpacked-extensions branch from 694e746 to 5c2e693 Compare January 3, 2019 19:07

lguychard added 5 commits January 7, 2019 12:11

Add new class ExtensionStorageSubject

b9dae55

ExtensionStorageSubject is an RxJS subject backed by an extension storage item

Add unpackedExtensionURL to platformContext

faf1abd

Load unpacked extension when unpackedExtensionURL emits

c529d4c

Edit unpackedExtensionURL from ExtensionStatus

d1c1757

styling

692d9cc

lguychard and others added 3 commits January 7, 2019 12:11

Add tests

67db8cb

Remove only

dbd3b63

Update shared/src/extensions/ExtensionStatus.tsx

788176c

Co-Authored-By: lguychard <l.guychard@gmail.com>

lguychard force-pushed the unpacked-extensions branch from 5c2e693 to fdd8a58 Compare January 7, 2019 12:39

PR comments

23696af

lguychard force-pushed the unpacked-extensions branch from fdd8a58 to 23696af Compare January 7, 2019 13:05

lguychard merged commit 0fc289a into master Jan 7, 2019

lguychard deleted the unpacked-extensions branch January 14, 2019 16:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Loading unpacked extensions #1508

Loading unpacked extensions #1508

lguychard commented Dec 19, 2018 •

edited

Loading

sqs left a comment

codecov-io commented Jan 2, 2019 •

edited

Loading

lguychard commented Jan 2, 2019

ijsnow Jan 2, 2019

sqs left a comment •

edited

Loading

sqs Jan 3, 2019

lguychard Jan 3, 2019

sqs Jan 3, 2019

lguychard Jan 3, 2019

	placeholder="URL"
	placeholder="URL to extension's package.json"

Loading unpacked extensions #1508

Loading unpacked extensions #1508

Conversation

lguychard commented Dec 19, 2018 • edited Loading

sqs left a comment

Choose a reason for hiding this comment

codecov-io commented Jan 2, 2019 • edited Loading

Codecov Report

lguychard commented Jan 2, 2019

ijsnow Jan 2, 2019

Choose a reason for hiding this comment

sqs left a comment • edited Loading

Choose a reason for hiding this comment

sqs Jan 3, 2019

Choose a reason for hiding this comment

lguychard Jan 3, 2019

Choose a reason for hiding this comment

sqs Jan 3, 2019

Choose a reason for hiding this comment

lguychard Jan 3, 2019

Choose a reason for hiding this comment

lguychard commented Dec 19, 2018 •

edited

Loading

codecov-io commented Jan 2, 2019 •

edited

Loading

sqs left a comment •

edited

Loading