-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Shared UX] Migrate router from kibana react to shared ux #138544
Merged
Merged
Changes from all commits
Commits
Show all changes
44 commits
Select commit
Hold shift + click to select a range
407ac63
router migration
rshen91 4947212
update bazel
rshen91 c840952
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine ba819b6
Merge remote-tracking branch 'upstream/main' into router-migrate
rshen91 63c5fd3
Merge remote-tracking branch 'origin/router-migrate' into router-migrate
rshen91 9fda52d
some doc updates
rshen91 e86adf1
Merge remote-tracking branch 'upstream/main' into router-migrate
rshen91 ac912df
make impl folder
rshen91 bb741e0
Merge remote-tracking branch 'upstream/main' into router-migrate
rshen91 cac3d71
missed files
rshen91 1ee73ef
Merge remote-tracking branch 'upstream/main' into router-migrate
rshen91 e0d159a
Merge remote-tracking branch 'upstream/main' into router-migrate
rshen91 be43990
remove merge conflict issues
rshen91 ab2daeb
update
rshen91 34a6ee3
[CI] Auto-commit changed files from 'node scripts/generate packages_b…
kibanamachine 4612e08
fix
rshen91 ee65d0d
Merge remote-tracking branch 'origin/router-migrate' into router-migrate
rshen91 37df4aa
move bazel files into impl directory
rshen91 094b980
Adjust BAZEL.build settings; fix types package
clintandrewhall bd2e6e4
Merge remote-tracking branch 'upstream/main' into router-migrate
rshen91 24e8ced
Merge remote-tracking branch 'origin/router-migrate' into router-migrate
rshen91 e9896f1
create mock package for router
rshen91 7bcea9d
storybook support mdx
rshen91 7d66fd6
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine 278f93c
Merge branch 'main' into router-migrate
rshen91 4255db7
fix mdx import issue
rshen91 00f763b
Merge remote-tracking branch 'origin/router-migrate' into router-migrate
rshen91 7dfe755
Merge remote-tracking branch 'upstream/main' into router-migrate
rshen91 6f0a92b
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine a03d3c0
Merge remote-tracking branch 'upstream/main' into router-migrate
rshen91 35c3a9d
Merge remote-tracking branch 'origin/router-migrate' into router-migrate
rshen91 3a69a1c
update
rshen91 7a1bc8c
Merge branch 'main' into router-migrate
rshen91 abd6d38
Merge remote-tracking branch 'upstream/main' into router-migrate
rshen91 e301437
Merge remote-tracking branch 'origin/router-migrate' into router-migrate
rshen91 a31f7b1
improve documentation
rshen91 7f1cc68
update tests
rshen91 671ad52
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine dc0b795
fix location key from rewriting
rshen91 6910e41
Merge remote-tracking branch 'origin/router-migrate' into router-migrate
rshen91 0556d16
apply ternary
rshen91 2443bb8
Merge branch 'main' into router-migrate
rshen91 b6e859f
[CI] Auto-commit changed files from 'node scripts/generate packages_b…
kibanamachine 822e7d6
Merge branch 'main' into router-migrate
rshen91 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,140 @@ | ||
load("@npm//@bazel/typescript:index.bzl", "ts_config") | ||
load("@build_bazel_rules_nodejs//:index.bzl", "js_library") | ||
load("//src/dev/bazel:index.bzl", "jsts_transpiler", "pkg_npm", "pkg_npm_types", "ts_project") | ||
|
||
PKG_DIRNAME = "shared-ux-router" | ||
PKG_REQUIRE_NAME = "@kbn/shared-ux-router" | ||
|
||
SOURCE_FILES = glob( | ||
[ | ||
"**/*.ts", | ||
"**/*.tsx", | ||
"**/*.mdx" | ||
], | ||
exclude = [ | ||
"**/*.test.*", | ||
], | ||
) | ||
|
||
SRCS = SOURCE_FILES | ||
|
||
filegroup( | ||
name = "srcs", | ||
srcs = SRCS, | ||
) | ||
|
||
NPM_MODULE_EXTRA_FILES = [ | ||
"package.json", | ||
] | ||
|
||
# In this array place runtime dependencies, including other packages and NPM packages | ||
# which must be available for this code to run. | ||
# | ||
# To reference other packages use: | ||
# "//repo/relative/path/to/package" | ||
# eg. "//packages/kbn-utils" | ||
# | ||
# To reference a NPM package use: | ||
# "@npm//name-of-package" | ||
# eg. "@npm//lodash" | ||
RUNTIME_DEPS = [ | ||
"@npm//react", | ||
"@npm//react-router-dom", | ||
"@npm//react-use", | ||
"@npm//rxjs", | ||
"//packages/kbn-shared-ux-utility", | ||
"//packages/kbn-test-jest-helpers", | ||
] | ||
|
||
# In this array place dependencies necessary to build the types, which will include the | ||
# :npm_module_types target of other packages and packages from NPM, including @types/* | ||
# packages. | ||
# | ||
# To reference the types for another package use: | ||
# "//repo/relative/path/to/package:npm_module_types" | ||
# eg. "//packages/kbn-utils:npm_module_types" | ||
# | ||
# References to NPM packages work the same as RUNTIME_DEPS | ||
TYPES_DEPS = [ | ||
"@npm//@types/node", | ||
"@npm//@types/jest", | ||
"@npm//@types/react", | ||
"@npm//@types/react-router-dom", | ||
"@npm//react-use", | ||
"@npm//rxjs", | ||
"//packages/kbn-shared-ux-utility:npm_module_types", | ||
"//packages/shared-ux/router/types:npm_module_types", | ||
"//packages/kbn-ambient-ui-types", | ||
] | ||
|
||
jsts_transpiler( | ||
name = "target_node", | ||
srcs = SRCS, | ||
build_pkg_name = package_name(), | ||
) | ||
|
||
jsts_transpiler( | ||
name = "target_web", | ||
srcs = SRCS, | ||
build_pkg_name = package_name(), | ||
web = True, | ||
additional_args = [ | ||
"--copy-files", | ||
"--quiet" | ||
], | ||
) | ||
|
||
ts_config( | ||
name = "tsconfig", | ||
src = "tsconfig.json", | ||
deps = [ | ||
"//:tsconfig.base.json", | ||
"//:tsconfig.bazel.json", | ||
], | ||
) | ||
|
||
ts_project( | ||
name = "tsc_types", | ||
args = ['--pretty'], | ||
srcs = SRCS, | ||
deps = TYPES_DEPS, | ||
declaration = True, | ||
declaration_map = True, | ||
emit_declaration_only = True, | ||
out_dir = "target_types", | ||
tsconfig = ":tsconfig", | ||
) | ||
|
||
js_library( | ||
name = PKG_DIRNAME, | ||
srcs = NPM_MODULE_EXTRA_FILES, | ||
deps = RUNTIME_DEPS + [":target_node", ":target_web"], | ||
package_name = PKG_REQUIRE_NAME, | ||
visibility = ["//visibility:public"], | ||
) | ||
|
||
pkg_npm( | ||
name = "npm_module", | ||
deps = [":" + PKG_DIRNAME], | ||
) | ||
|
||
filegroup( | ||
name = "build", | ||
srcs = [":npm_module"], | ||
visibility = ["//visibility:public"], | ||
) | ||
|
||
pkg_npm_types( | ||
name = "npm_module_types", | ||
srcs = SRCS, | ||
deps = [":tsc_types"], | ||
package_name = PKG_REQUIRE_NAME, | ||
tsconfig = ":tsconfig", | ||
visibility = ["//visibility:public"], | ||
) | ||
|
||
filegroup( | ||
name = "build_types", | ||
srcs = [":npm_module_types"], | ||
visibility = ["//visibility:public"], | ||
) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
--- | ||
id: sharedUX/Router | ||
slug: /shared-ux/router | ||
title: Router | ||
description: A router component | ||
tags: ['shared-ux', 'component', 'router', 'route'] | ||
date: 2022-08-12 | ||
--- | ||
|
||
## Summary | ||
This is a wrapper around the `react-router-dom` Route component that inserts MatchPropagator in every application route. It helps track all route changes and send them to the execution context, later used to enrich APM 'route-change' transactions. | ||
The component does not require any props and accepts props from the RouteProps interface such as a `path`, or a component like `AppContainer`. | ||
|
||
|
||
```jsx | ||
<Route path="/url" /> | ||
``` | ||
|
||
### Explanation of RouteProps | ||
|
||
```jsx | ||
export interface RouteProps { | ||
location?: H.Location; | ||
component?: React.ComponentType<RouteComponentProps<any>> | React.ComponentType<any>; | ||
render?: (props: RouteComponentProps<any>) => React.ReactNode; | ||
children?: ((props: RouteChildrenProps<any>) => React.ReactNode) | React.ReactNode; | ||
path?: string | string[]; | ||
exact?: boolean; | ||
sensitive?: boolean; | ||
strict?: boolean; | ||
} | ||
``` | ||
|
||
All props are optional | ||
|
||
| Prop Name | Prop Type | Description | | ||
|---|---|---| | ||
| `location` | `H.Location` | the location of one instance of history | | ||
| `component` | `React.ComponentType<RouteComponentProps<any>>` or `React.ComponentType<any>;` | a react component | | ||
| `render` | `(props: RouteComponentProps<any>) => React.ReactNode;` | render props to a react node| | ||
| `children` | `((props: RouteChildrenProps<any>) => React.ReactNode)` or `React.ReactNode;` | pass children to a react node | | ||
| `path` | `string` or `string[];` | a url path or array of paths | | ||
| `exact` | `boolean` | exact match for a route (see: https://stackoverflow.com/questions/52275146/usage-of-exact-and-strict-props) | | ||
| `sensitive` | `boolean` | case senstive route | | ||
| `strict` | `boolean` | strict entry of the requested path in the path name | | ||
|
||
|
||
|
||
This component removes the need for manual calls to `useExecutionContext` and they should be removed. | ||
|
||
## EUI Promotion Status | ||
|
||
This component is not currently considered for promotion to EUI. |
35 changes: 35 additions & 0 deletions
35
packages/shared-ux/router/impl/__snapshots__/router.test.tsx.snap
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
export { Route } from './router'; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
module.exports = { | ||
preset: '@kbn/test', | ||
rootDir: '../../../..', | ||
roots: ['<rootDir>/packages/shared-ux/router/impl'], | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
{ | ||
"name": "@kbn/shared-ux-router", | ||
"private": true, | ||
"version": "1.0.0", | ||
"main": "./target_node/index.js", | ||
"browser": "./target_web/index.js", | ||
"license": "SSPL-1.0 OR Elastic License 2.0" | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
import React, { Component, FC } from 'react'; | ||
import { shallow } from 'enzyme'; | ||
import { Route } from './router'; | ||
import { createMemoryHistory } from 'history'; | ||
|
||
describe('Route', () => { | ||
test('renders', () => { | ||
const example = shallow(<Route />); | ||
expect(example).toMatchSnapshot(); | ||
}); | ||
|
||
test('location renders as expected', () => { | ||
// create a history | ||
const historyLocation = createMemoryHistory(); | ||
// add the path to the history | ||
historyLocation.push('/app/wow'); | ||
// prevent the location key from remaking itself each jest test | ||
historyLocation.location.key = 's5brde'; | ||
// the Route component takes the history location | ||
const example = shallow(<Route location={historyLocation.location} />); | ||
expect(example).toMatchSnapshot(); | ||
}); | ||
|
||
test('component prop renders', () => { | ||
const sampleComponent: FC<{}> = () => { | ||
return <Component>Test</Component>; | ||
}; | ||
const example = shallow(<Route component={sampleComponent} />); | ||
expect(example).toMatchSnapshot(); | ||
}); | ||
|
||
test('render prop renders', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nice! |
||
const sampleReactNode = React.createElement('li', { id: 'li1' }, 'one'); | ||
const example = shallow(<Route render={() => sampleReactNode} />); | ||
expect(example).toMatchSnapshot(); | ||
}); | ||
}); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a more extensive test suite? Given that the render logic depends on a lot of props, would be nice to test some of those scenarios.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added some of the react dom route props in the test suite - let me know what you think. I think for testing
exact
,sensitive
andstrict
it might involve an integration test?