Skip to content
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

Restrict direct Route imports from react-router-dom #145863

Closed
wants to merge 1 commit into from

Conversation

xcrzx
Copy link
Contributor

@xcrzx xcrzx commented Nov 21, 2022

Related to: #132629

Summary

TBD

@xcrzx xcrzx self-assigned this Nov 21, 2022
@xcrzx xcrzx added 8.7 candidate release_note:skip Skip the PR/issue when compiling release notes labels Nov 21, 2022
@xcrzx xcrzx force-pushed the router-with-context-propagation branch from e936f39 to c36a966 Compare November 21, 2022 14:25
@kibana-ci
Copy link
Collaborator

kibana-ci commented Nov 21, 2022

💔 Build Failed

Failed CI Steps

Metrics [docs]

Unknown metric groups

ESLint disabled in files

id before after diff
osquery 1 2 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
fleet 59 65 +6
osquery 109 115 +6
securitySolution 442 448 +6
total +20

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
fleet 67 73 +6
osquery 110 117 +7
securitySolution 519 525 +6
total +21

History

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

cc @xcrzx

mistic added a commit that referenced this pull request Feb 14, 2023
## Summary

This PR removes all imports of Route from react-router-dom and
'@kbn/kibana-react-plugin/public' and instead imports Route from
@kbn/shared-ux-router.

### Context
Based on
#132629 (comment) This PR
executes steps 2 - 4:

> 2. To make the transition easier, we want to re-export other
react-router-dom exports alongside the modified' Route'.
> 3. Solutions should start using that Route component in place of the
one from react-router-dom. I.e. replace all occurrences of import { ...
} from 'react-router-dom' with import { ... } from
'@kbn/shared-ux-router'.
> 4. All manual calls to useExecutionContext are not needed anymore and
should be removed.

### Future PR

Looks like this might be getting worked on in:
#145863 (thanks!)

> Introduce an ESlint rule that ensures that react-router-dom is not
used directly in Kibana and that imports go through the new
@kbn/shared-ux-router package.

This is tangentially accomplished through
#150340 but only addresses using
Route through @kbn/kibana-react-plugin/public'


### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Tiago Costa <[email protected]>
justinkambic pushed a commit to justinkambic/kibana that referenced this pull request Feb 23, 2023
## Summary

This PR removes all imports of Route from react-router-dom and
'@kbn/kibana-react-plugin/public' and instead imports Route from
@kbn/shared-ux-router.

### Context
Based on
elastic#132629 (comment) This PR
executes steps 2 - 4:

> 2. To make the transition easier, we want to re-export other
react-router-dom exports alongside the modified' Route'.
> 3. Solutions should start using that Route component in place of the
one from react-router-dom. I.e. replace all occurrences of import { ...
} from 'react-router-dom' with import { ... } from
'@kbn/shared-ux-router'.
> 4. All manual calls to useExecutionContext are not needed anymore and
should be removed.

### Future PR

Looks like this might be getting worked on in:
elastic#145863 (thanks!)

> Introduce an ESlint rule that ensures that react-router-dom is not
used directly in Kibana and that imports go through the new
@kbn/shared-ux-router package.

This is tangentially accomplished through
elastic#150340 but only addresses using
Route through @kbn/kibana-react-plugin/public'


### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Tiago Costa <[email protected]>
@xcrzx xcrzx closed this Feb 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.7 candidate release_note:skip Skip the PR/issue when compiling release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants