Routing example plugin #69581

stacey-gammon · 2020-06-18T20:47:46Z

Add an example plugin that shows some basic routing examples.

I still need to add tests but first want buy in on what is showcased from the @elastic/kibana-platform team

pgayvallet

Some nits, did not review the react components code.

Overall this seems like a good example plugin.

pgayvallet · 2020-06-23T11:58:21Z

examples/routing_example/server/index.ts

+
+import { RoutingExamplePlugin } from './plugin';
+
+export const plugin: PluginInitializer<void, void> = () => new RoutingExamplePlugin();


NIT: returning void as a contract got subtle issues, such as not being able to check if the plugin is enabled or not when used as an optional dependency from another plugin.

We really need to educate to use {} instead, so having it in our test plugins would be great.

will do. There are other example plugins that use void. Couldn't this be type checked? TSetup extends object = object,

Yea, the default is void

export interface Plugin< TSetup = void, TStart = void, TPluginsSetup extends object = object, TPluginsStart extends object = object > {

which makes it actually seem like void is supposed to be the default, not {}.

There are other example plugins that use void. Couldn't this be type checked?

Yea, the current default is void. And even some non-example plugins are using it.

TBH I tried a few weeks back to change it to extends object and adapt all usages, but this is way bigger than is looks due to the amount of plugins, and this type is using in some sensible TS magic in core, causing lots of TS errors.

You can keep void for now if you want, as we'll have to fix all the plugins at once when we'll perform this change.

@restrry @joshdover Before I open an actual issue, do we all agree that plugins should always return a contract (even empty) instead of void?

examples/routing_example/server/plugin.ts

examples/routing_example/server/routes/random_number_generator.ts

pgayvallet · 2020-06-23T12:05:56Z

examples/routing_example/public/services.ts

+      } catch (e) {
+        return e;
+      }


should we remove the catch(e) throw e blocks? Or do we want them to show that fetch throws in case of code >= 400 ?

I wanted consumers to be forced to handle the possibility of an error, which is explicit when the return is Response | Error not just Response.

But, I don't know, I'm also torn because then consumers need to do the isError(response) check which isn't that nice either.

Oh sorry, I misread that for a catch/throw instead of catch/return.

FYI, platform is going to open a discussion issue on this subject (should core APIs throw errors or return Either<success, error>-ish monads)

examples/routing_example/public/index.ts

examples/routing_example/public/is_error.ts

…uting-examples

…n/kibana into 2020-06-18-routing-examples

…uting-examples

stacey-gammon · 2020-06-25T13:58:45Z

Added tests.

stacey-gammon · 2020-07-09T13:36:59Z

@elasticmachine merge upstream

…uting-examples

stacey-gammon · 2020-07-09T20:09:10Z

ping @elastic/kibana-platform

stacey-gammon · 2020-07-13T12:35:37Z

@elasticmachine merge upstream

stacey-gammon · 2020-07-14T11:26:12Z

@elasticmachine merge upstream

pgayvallet

LGTM

pgayvallet · 2020-07-14T12:31:08Z

examples/routing_example/public/app.tsx

+            <EuiListGroup
+              listItems={[
+                {
+                  label: 'IRouter API docs',


I guess i18n is out of the scope of the example?

I'm torn on it. None of the examples use translations right now.

I don't think these will ever actually be translated because examples aren't in the distributable (though I could be wrong. @Bamieh - do you know if there is a way to run via yarn start and specify a different language, and have these translations that aren't in the distributable actually translated?).

If you could run it with a different language, it'd be helpful for developers who want to read the examples in a different language (although all of our code comments and READMEs are in English).

It'd be good to have them so copy/pastes will include translations, but this probably wouldn't be code that would be copy/pasted. If a third party developer writes an app and uses these translation services, their code still won't be translated. They need to do extra work.

It's a good question, but until I have clarity, I'm going to leave as is, at least for code that isn't meant to be copy/pasted.

examples/routing_example/public/is_error.ts

…uting-examples

stacey-gammon · 2020-07-14T20:25:30Z

@elasticmachine merge upstream

kibanamachine · 2020-07-14T22:09:27Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 47ae774

Build metrics

✅ unchanged

History

💔 Build #61564 failed 3913378
💔 Build #61476 failed 99d7423
💔 Build #61398 failed 0441d4c
💔 Build #61365 failed 0efbc1e
💚 Build #61319 succeeded 6594ab4

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

* Routing example plugin * Routing example plugin * address review comments * consolidate route registration into single function * ts fix * Add functional tests * typescript fix * Fix typo * check against HttpFetchError not Error * fix ts * fix unhappy ci Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

* Routing example plugin * Routing example plugin * address review comments * consolidate route registration into single function * ts fix * Add functional tests * typescript fix * Fix typo * check against HttpFetchError not Error * fix ts * fix unhappy ci Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

stacey-gammon added release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.9.0 v8.0.0 labels Jun 18, 2020

Routing example plugin

ebd5055

stacey-gammon force-pushed the 2020-06-18-routing-examples branch from 808aa62 to ebd5055 Compare June 18, 2020 20:48

Routing example plugin

45380cf

pgayvallet reviewed Jun 23, 2020

View reviewed changes

stacey-gammon added 7 commits June 23, 2020 08:52

address review comments

df0345c

Merge branch 'master' of github.com:elastic/kibana into 2020-06-18-ro…

1e77bfc

…uting-examples

Merge branch '2020-06-18-routing-examples' of github.com:stacey-gammo…

2db71dc

…n/kibana into 2020-06-18-routing-examples

consolidate route registration into single function

71de903

ts fix

76555a9

Merge branch 'master' of github.com:elastic/kibana into 2020-06-18-ro…

954249e

…uting-examples

Add functional tests

d0cf5bb

pgayvallet mentioned this pull request Jun 25, 2020

Plugin lifecycle methods that return undefined block dependents from inferring the plugin's disabled state #69845

Closed

stacey-gammon added 2 commits June 25, 2020 07:48

typescript fix

92b3dc9

Fix typo

4030ba4

elasticmachine and others added 2 commits July 9, 2020 07:37

Merge branch 'master' into 2020-06-18-routing-examples

5986909

Merge branch 'master' of github.com:elastic/kibana into 2020-06-18-ro…

c7e1e61

…uting-examples

stacey-gammon requested review from pgayvallet and mshustov July 13, 2020 12:35

Merge branch 'master' into 2020-06-18-routing-examples

6594ab4

pgayvallet approved these changes Jul 14, 2020

View reviewed changes

check against HttpFetchError not Error

0efbc1e

stacey-gammon added 7.10.0 and removed v7.9.0 labels Jul 14, 2020

stacey-gammon added 3 commits July 14, 2020 10:14

fix ts

0441d4c

Merge branch 'master' of github.com:elastic/kibana into 2020-06-18-ro…

99d7423

…uting-examples

fix unhappy ci

3913378

Merge branch 'master' into 2020-06-18-routing-examples

47ae774

spalger added v7.10.0 and removed 7.10.0 labels Jul 15, 2020

stacey-gammon merged commit 339e13b into elastic:master Jul 15, 2020

stacey-gammon mentioned this pull request Jul 15, 2020

[7.x] Routing example plugin (#69581) #71901

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Routing example plugin #69581

Routing example plugin #69581

stacey-gammon commented Jun 18, 2020 •

edited

Loading

pgayvallet left a comment

pgayvallet Jun 23, 2020

stacey-gammon Jun 23, 2020

stacey-gammon Jun 23, 2020

pgayvallet Jun 24, 2020

pgayvallet Jun 23, 2020

stacey-gammon Jun 23, 2020

pgayvallet Jun 24, 2020

stacey-gammon commented Jun 25, 2020

stacey-gammon commented Jul 9, 2020

stacey-gammon commented Jul 9, 2020

stacey-gammon commented Jul 13, 2020

stacey-gammon commented Jul 14, 2020

pgayvallet left a comment

pgayvallet Jul 14, 2020

stacey-gammon Jul 14, 2020

stacey-gammon commented Jul 14, 2020

kibanamachine commented Jul 14, 2020


		import { RoutingExamplePlugin } from './plugin';

		export const plugin: PluginInitializer<void, void> = () => new RoutingExamplePlugin();

Routing example plugin #69581

Routing example plugin #69581

Conversation

stacey-gammon commented Jun 18, 2020 • edited Loading

pgayvallet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stacey-gammon commented Jun 25, 2020

stacey-gammon commented Jul 9, 2020

stacey-gammon commented Jul 9, 2020

stacey-gammon commented Jul 13, 2020

stacey-gammon commented Jul 14, 2020

pgayvallet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stacey-gammon commented Jul 14, 2020

kibanamachine commented Jul 14, 2020

💚 Build Succeeded

Build metrics

History

stacey-gammon commented Jun 18, 2020 •

edited

Loading