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(RSC): Remove entries.ts file #10533

Merged
merged 6 commits into from
May 3, 2024
Merged

feat(RSC): Remove entries.ts file #10533

merged 6 commits into from
May 3, 2024

Conversation

Josh-Walker-GM
Copy link
Collaborator

@Josh-Walker-GM Josh-Walker-GM commented May 2, 2024

Problem
We currently require you to explicitly map certain names to import paths. For example between page names and page import paths. This is later used at dev or build time to stitch things together properly. This isn't great DX as we make the user essentially duplicate information that is present/known from the router.

Notes

  1. I didn't attempt to alter our current serve vs dev support. Code which relies on the project having been built still does so and code which acted differently for serve vs dev still does so.

Outstanding
1. This PR has a side effect of reserving the ServerEntry name. Adding a page called that would potentially cause issues as it would be overridden in the getEntries function. We renamed it to __rwjs__ServerEntry to mitigate this

@Josh-Walker-GM Josh-Walker-GM added release:feature This PR introduces a new feature changesets-ok Override the changesets check labels May 2, 2024
@Josh-Walker-GM Josh-Walker-GM added this to the RSC milestone May 2, 2024
@Josh-Walker-GM Josh-Walker-GM self-assigned this May 2, 2024
@Josh-Walker-GM Josh-Walker-GM marked this pull request as ready for review May 2, 2024 17:24
@Josh-Walker-GM Josh-Walker-GM requested a review from Tobbe May 2, 2024 17:24
packages/vite/src/lib/entries.ts Outdated Show resolved Hide resolved
packages/vite/src/rsc/rscBuildAnalyze.ts Outdated Show resolved Hide resolved
packages/vite/src/rsc/rscBuildForServer.ts Outdated Show resolved Hide resolved
packages/vite/src/lib/entries.ts Outdated Show resolved Hide resolved
@Josh-Walker-GM Josh-Walker-GM requested a review from Tobbe May 2, 2024 23:24
packages/vite/src/rsc/rscBuildAnalyze.ts Outdated Show resolved Hide resolved
packages/vite/src/rsc/rscBuildEntriesFile.ts Outdated Show resolved Hide resolved
packages/vite/src/rsc/rscBuildEntriesFile.ts Show resolved Hide resolved
packages/vite/src/clientSsr.ts Outdated Show resolved Hide resolved
Copy link
Member

@Tobbe Tobbe left a comment

Choose a reason for hiding this comment

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

LGTM

@Tobbe Tobbe enabled auto-merge (squash) May 3, 2024 06:11
@Tobbe Tobbe merged commit f1a3c1c into main May 3, 2024
46 checks passed
@Tobbe Tobbe deleted the jgmw/rsc-remove-entries-file branch May 3, 2024 06:22
dac09 added a commit to dac09/redwood that referenced this pull request May 3, 2024
…dwood into feat/supabase-middleware-client

* 'feat/supabase-middleware-client' of github.com:dac09/redwood:
  feat(RSC): Remove `entries.ts` file (redwoodjs#10533)
  testing to see if getToken is called
  Try to see if swapping getToken order mock passes Windows CI
  feat(server-auth): Refactor useReauthenticate to prevent double currentUser calls (redwoodjs#10531)
  chore(deps): update dependency rollup to v4.17.2 (redwoodjs#10346)
  fix(deps): update docusaurus monorepo to v3.2.1 (redwoodjs#10371)
Josh-Walker-GM added a commit that referenced this pull request May 7, 2024
#10549)

**Problem**
This is a follow up to #10533 which removed the need for the
`entries.ts` file. That PR failed to remove the file from both the
existing RSC test fixtures and the setup command.

**Changes**
1. Removes the `entries.ts` file from the RSC test fixtures
2. Removes the step in the RSC setup command that adds the `entries.ts`
file to a project.
@Josh-Walker-GM Josh-Walker-GM modified the milestones: RSC, v8.0.0 Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changesets-ok Override the changesets check release:feature This PR introduces a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants