Minor improvements/fixes to custom plugin API doc #13358
Conversation
Found these by going through the steps in the recently updated documentation reflecting the new-and-improved mechanics for contributing custom plugin API.
| return { | ||
| fooBar: { | ||
| getFoo(): Promise<Foo> { | ||
| return fooExt.getFooImpl(); | ||
| return self.fooExt.getFooImpl(); |
There was a problem hiding this comment.
Explanation: you can't directly access fooExt from here. You need the self local above.
| @@ -31,7 +31,6 @@ import { ApiFactory, PluginContainerModule } from '@theia/plugin-ext/lib/plugin/ | |||
|
|
|||
| type Gotd = typeof gotd; | |||
| const GotdApiFactory = Symbol('GotdApiFactory'); | |||
| type GotdApiFactory = ApiFactory<Gotd>; | |||
There was a problem hiding this comment.
Explanation: this type seems extraneous.
There was a problem hiding this comment.
So it is! There's no problem of unused name reported by tsc/eslint because the const symbol of the same name is referenced.
| @@ -89,7 +89,6 @@ import { FooExtImpl } from './foo-ext-impl'; | |||
| import * as fooBarAPI from '@bar/foo'; | |||
|
|
|||
| type FooBarApi = typeof fooBarAPI; | |||
| type Foo = FooBarApi['Foo']; | |||
There was a problem hiding this comment.
This type is used at line 132.
There was a problem hiding this comment.
Indeed. I apparently got myself confused there. Will fix
| @@ -31,7 +31,6 @@ import { ApiFactory, PluginContainerModule } from '@theia/plugin-ext/lib/plugin/ | |||
|
|
|||
| type Gotd = typeof gotd; | |||
| const GotdApiFactory = Symbol('GotdApiFactory'); | |||
| type GotdApiFactory = ApiFactory<Gotd>; | |||
There was a problem hiding this comment.
So it is! There's no problem of unused name reported by tsc/eslint because the const symbol of the same name is referenced.
| @@ -89,7 +89,6 @@ import { FooExtImpl } from './foo-ext-impl'; | |||
| import * as fooBarAPI from '@bar/foo'; | |||
|
|
|||
| type FooBarApi = typeof fooBarAPI; | |||
| type Foo = FooBarApi['Foo']; | |||
|
|
|||
| const FooBarApiFactory = Symbol('FooBarApiFactory'); | |||
| type FooBarApiFactory = ApiFactory<FooBarApi>; | |||
There was a problem hiding this comment.
Similarly to the GotD example, this type definition is actually unused. We can remove it and the ApiFactory import as in GotD.
There was a problem hiding this comment.
Above you pointed out it's used at line 132, and that does appear to be the case. So am I wrong about being wrong?
There was a problem hiding this comment.
I guess part of the confusion is that both snippets are actually in the same file, and thus the contents of the file are spread out in two disjointed parts of the documentation.
There was a problem hiding this comment.
No, the FooBarApiFactory type definition doesn't appear to be used. The thing that's used at line 132 is the Foo type definition.
There was a problem hiding this comment.
So, where I got confused is that I mis-associated your comment here. My eye went to the pink-highlighted line (92) instead of the line directly above your comment (95). I should probably get more coffee. All is good. I'll fix the one issue and remove this line, too
There was a problem hiding this comment.
And we also need to remove the now-unused import of ApiFactory from the Foo example in the MD as we did for the GotD example in "real code".
|
OK. Hopefully it's all straight now. Thanks for the review. |
What it does
Corrects/improves a few things in documentation and in one of the example Theia Extensions.
Found these by going through the steps in the recently updated documentation reflecting the new-and-improved mechanics for contributing custom plugin API.
How to test
Review checklist
Reminder for reviewers