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

feat: Reworks RSC server entries and route manifest building to derive from routes and include if route info related to authentication #10572

Merged
merged 22 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
ecb5faf
Builds server entries from routes rather than pages
dthyresson May 15, 2024
3e42a4c
A route is private if it has a parent PrivateSet
dthyresson May 15, 2024
2b6ebf1
Adds isPrivate attribute to Route manifest
dthyresson May 15, 2024
674a725
Add changeset
dthyresson May 15, 2024
dfdea38
Merge branch 'main' into dt-refactor-server-entries-from-routes
dthyresson May 15, 2024
6416eba
Favor PrivateSet over Private in fixtures to authenticate routes. Tes…
dthyresson May 15, 2024
569b510
Adds unauthenticated path to RWRoute model
dthyresson May 16, 2024
74a9694
private redirect fix
dthyresson May 16, 2024
f7e35e8
update route manifest
dthyresson May 16, 2024
01f47f4
Merge branch 'main' into dt-refactor-server-entries-from-routes
dthyresson May 16, 2024
0f5b4f2
Had extraneous exp typo import
dthyresson May 16, 2024
9e9f062
remove console
dthyresson May 16, 2024
400580f
Add roles to building route manifest
dthyresson May 16, 2024
92659c3
Merge branch 'main' into dt-refactor-server-entries-from-routes
dthyresson May 16, 2024
778929a
improve changeset
dthyresson May 16, 2024
0ace4af
Merge branch 'main' into dt-refactor-server-entries-from-routes
dthyresson May 16, 2024
2e7bf8b
Merge branch 'main' into dt-refactor-server-entries-from-routes
dthyresson May 16, 2024
ba02c65
Merge branch 'main' into dt-refactor-server-entries-from-routes
dthyresson May 17, 2024
4584164
fix windows ci?
dthyresson May 17, 2024
6418794
Rename RWPage's const to constName
dthyresson May 21, 2024
7184362
Merge branch 'main' into dt-refactor-server-entries-from-routes
dthyresson May 21, 2024
a2ac9fc
Merge branch 'main' into dt-refactor-server-entries-from-routes
dthyresson May 21, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/internal/src/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,15 @@ export interface RWRouteManifestItem {
hasParams: boolean
relativeFilePath: string
redirect: { to: string; permanent: boolean } | null
isPrivate: boolean
// Probably want isNotFound here, so we can attach a separate 404 handler
}

export interface RouteSpec extends RWRouteManifestItem {
id: string
isNotFound: boolean
filePath: string | undefined
isPrivate: boolean
relativeFilePath: string
}

Expand Down Expand Up @@ -111,6 +113,7 @@ export const getProjectRoutes = (): RouteSpec[] => {
redirect: route.redirect
? { to: route.redirect, permanent: false }
: null,
isPrivate: route.isPrivate,
}
})
}
2 changes: 1 addition & 1 deletion packages/structure/src/model/RWRoute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export class RWRoute extends BaseNode {
?.getOpeningElement()
?.getTagNameNode()
?.getText()
return tagText === 'Private'
return tagText === 'Private' || tagText === 'PrivateSet'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need to keep "Private"? I think it has to be "PrivateSet" now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say lets remove Private in a separate PR, and remove it from exports too!

}

@lazy() get hasParameters(): boolean {
Expand Down
1 change: 1 addition & 0 deletions packages/vite/src/buildRouteManifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export async function buildRouteManifest() {
}
: null,
relativeFilePath: route.relativeFilePath,
isPrivate: route.isPrivate,
}

return acc
Expand Down
39 changes: 13 additions & 26 deletions packages/vite/src/lib/entries.ts
Original file line number Diff line number Diff line change
@@ -1,37 +1,24 @@
import path from 'node:path'

import type { PagesDependency } from '@redwoodjs/project-config'
import {
ensurePosixPath,
getPaths,
processPagesDir,
} from '@redwoodjs/project-config'
import { ensurePosixPath, getPaths } from '@redwoodjs/project-config'
import { getProject } from '@redwoodjs/structure/dist/index'
import type { RWPage } from '@redwoodjs/structure/dist/model/RWPage'
import type { RWRoute } from '@redwoodjs/structure/dist/model/RWRoute'

import { makeFilePath } from '../utils'

const getPathRelativeToSrc = (maybeAbsolutePath: string) => {
// If the path is already relative
if (!path.isAbsolute(maybeAbsolutePath)) {
return maybeAbsolutePath
}

return `./${path.relative(getPaths().web.src, maybeAbsolutePath)}`
}

const withRelativeImports = (page: PagesDependency) => {
return {
...page,
relativeImport: ensurePosixPath(getPathRelativeToSrc(page.importPath)),
}
}

export function getEntries() {
const entries: Record<string, string> = {}

// Build the entries object based on routes and pages
// Given the page's route, we can determine whether or not
// the entry requires authentication checks
const rwProject = getProject(getPaths().base)
const routes = rwProject.getRouter().routes

// Add the various pages
const pages = processPagesDir().map(withRelativeImports)
const pages = routes.map((route: RWRoute) => route.page) as RWPage[]

for (const page of pages) {
entries[page.importName] = page.path
entries[page.const_] = ensurePosixPath(page.path)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsure of the use of const_ is appropriate, but it generates the correct key.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this do again? Sorry can't remember 😅

Copy link
Contributor Author

@dthyresson dthyresson May 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use on const_ here provides the expected key for the entries file that is compatible with the prior way of serving the key:

  "UserExampleNewUserExamplePage": "assets/UserExampleNewUserExamplePage-CPG6dVwh.mjs",
  "UserExampleEditUserExamplePage": "assets/UserExampleEditUserExamplePage-BfvIJHxv.mjs",

UserExampleNewUserExamplePage is the key.

Before, the processPagesDir created a new type that enriches the page with and importName based on the glob file name:

export const processPagesDir = (
  webPagesDir: string = getPaths().web.pages,
): Array<PagesDependency> => {
  const pagePaths = fg.sync('**/*Page.{js,jsx,ts,tsx}', {
    cwd: webPagesDir,
    ignore: ['node_modules'],
  })
  return pagePaths.map((pagePath) => {
    const p = path.parse(pagePath)

    const importName = p.dir.replace(/\//g, '') <<<<<----
    const importPath = importStatementPath(
      path.join(webPagesDir, p.dir, p.name),

Whatever const_ is it appears to have the same info we need without having to process the paths.

I think it comes from structure model File or BaseNode.


// Add the ServerEntry entry, noting we use the "__rwjs__" prefix to avoid
Expand Down
Loading