Extract the Router APIs and context into a dedicated package #50100
Conversation
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
youknowriad
added
[Type] Code Quality
youknowriad
requested review from
ajitbohra,
ryanwelcher,
juanmaguitar and
fabiankaegy
as code owners
|
There's a couple of other "Router APIs" that we already have that are in the site editor (useTitle, useLink), they can probably be moved to this new package ultimately if we feel the need. |
|
Size Change: -919 B (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 0050788. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4808809928
|
kevin940726
approved these changes
Apr 27, 2023
| "react-native": "src/index", | ||
| "dependencies": { | ||
| "@babel/runtime": "^7.16.0", | ||
| "@wordpress/element": "file:../element", |
There was a problem hiding this comment.
Slightly off-topic: I know we have precedence but I think it makes more sense to mark @wordpress/element as a peer dependency than a dependency. Starting from a newer version of npm, it should automatically install peer dependency on npm install.
Also since that we're not directly importing from react, we don't need to make react as a peer dependency?
There was a problem hiding this comment.
Yeah, to be honest I just copied what we do in other packages. I agree that your proposal seems correct but I'd rather we do it in its own dedicated PR (probably after the npm upgrade too). cc @gziolo
There was a problem hiding this comment.
I agree that we need to rethink and document all the practices after we migrate Node and npm to the latest LTS version. For the time being, mirroring other packages is the best approach.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Pre-requisite for #49956
What?
This PR extracts the router provider and some related APIs (useHistory, useLocation) to a dedicated package in order to unblock #49956 allowing us to access the router/history from commands that are independent of the site editor.
How?
This is the exact same code and APIs we already have in the site editor. These APIs were not necessarily meant to be external APIs yet (even if I can see plugins wanting to actually use them). They are very naive, so I'm making all of them as private APIs for now.
Testing Instructions
Nothing really to test, just make sure you can open and navigate the site editor.