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

chore: Adds pageIdentifier to route manifest #10680

Merged
merged 5 commits into from
May 25, 2024
Merged

Conversation

dthyresson
Copy link
Contributor

@dthyresson dthyresson commented May 23, 2024

This PR adds the page_identifier_str of pageIdentifier to the Route Manifest.

Known what page belongs to the route can be useful to :

  • ensure if rendering a page/component that it belongs to the route and its auth permissions
  • for visualizing routes
  • general completeness in the manifest with the Routes jsx in manifest form

Example

Routes

const Routes = () => {
  return (
    <Router>
      <Set wrap={NavigationLayout}>
        <Route path="/" page={HomePage} name="home" />
        <Route path="/about" page={AboutPage} name="about" />
        <PrivateSet unauthenticated="home" roles={['admin']}>
          <Route path="/info" page={AboutPage} name="info" />
        </PrivateSet>
        <Route path="/multi-cell" page={MultiCellPage} name="multiCell" />

Manifest

{
  "/": {
    "name": "home",
    "bundle": null,
    "matchRegexString": "^/$",
    "pathDefinition": "/",
    "hasParams": false,
    "routeHooks": null,
    "redirect": null,
    "relativeFilePath": "pages/HomePage/HomePage.tsx",
    "isPrivate": false,
    "pageIdentifier": "HomePage"
  },
  "/about": {
    "name": "about",
    "bundle": null,
    "matchRegexString": "^/about$",
    "pathDefinition": "/about",
    "hasParams": false,
    "routeHooks": null,
    "redirect": null,
    "relativeFilePath": "pages/AboutPage/AboutPage.tsx",
    "isPrivate": false,
    "pageIdentifier": "AboutPage"
  },
  "/info": {
    "name": "info",
    "bundle": null,
    "matchRegexString": "^/info$",
    "pathDefinition": "/info",
    "hasParams": false,
    "routeHooks": null,
    "redirect": null,
    "relativeFilePath": "pages/AboutPage/AboutPage.tsx",
    "isPrivate": true,
    "unauthenticated": "home",
    "roles": [
      "admin"
    ],
    "pageIdentifier": "AboutPage"
  },
  "/multi-cell": {
    "name": "multiCell",
    "bundle": null,
    "matchRegexString": "^/multi-cell$",
    "pathDefinition": "/multi-cell",
    "hasParams": false,
    "routeHooks": null,
    "redirect": null,
    "relativeFilePath": "pages/MultiCellPage/MultiCellPage.tsx",
    "isPrivate": false,
    "pageIdentifier": "MultiCellPage"
  },

@dthyresson dthyresson added this to the RSC milestone May 23, 2024
@dthyresson dthyresson requested a review from dac09 May 23, 2024 20:27
@dthyresson dthyresson added the release:chore This PR is a chore (means nothing for users) label May 23, 2024
@dthyresson dthyresson changed the title feat: Adds pageIdentifier to route manifest chore: Adds pageIdentifier to route manifest May 23, 2024
@dthyresson dthyresson marked this pull request as ready for review May 23, 2024 20:27
@dac09
Copy link
Collaborator

dac09 commented May 25, 2024

Hey DT Looks good, but also need to update getProjectRoutes for the equivalent in dev. Sorry missed it, it does! All good ✌️

My Idea with the routeManifest and getPageRoutes was that we'd have a superset of a RouteManifestItem in dev!

@dac09 dac09 merged commit afda80e into main May 25, 2024
46 checks passed
@dac09 dac09 deleted the dt-add-page-to-route-manifest branch May 25, 2024 05:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:chore This PR is a chore (means nothing for users)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants