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.

Sign up for GitHub

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

Jump to bottom

fix(routes): Handle explicitly imported pages with different names #4948

Merged
merged 31 commits into from
Apr 2, 2022
Merged

fix(routes): Handle explicitly imported pages with different names #4948

merged 31 commits into from
Apr 2, 2022

Conversation

dac09
Copy link
Contributor

@dac09 dac09 commented Mar 28, 2022 •
edited by Tobbe
Loading

This PR fixes the issues linked below discovered on Sprout , the gist of it being the Route auto loader plugin was bugging out when you:
a) Nest a page in a folder

./web/src/pages/
├── Jobs
│   ├── NewJobPage
│   │   └── NewJobPage.tsx

b) Import that page with a slightly different name than expected
e.g.

import NewJobPage from 'src/pages/Jobs/NewJobPage'

// instead of the expected
import JobsNewJobPage from 'src/pages/Jobs/NewJobPage'

(all which is reasonable)

This PR does the following:

  • checks if the import has a custom name when prerendering, and preserves that name
  • simplifies the logic for handling relative/absolute paths which are a real pain
  • adds additional unit tests with a fixture that takes a snapshot of sprout to reproduce
  • adds prerender smoke tests (♥️ playwright) for when:
    i) A page is explicitly imported (HomePage)
    ii) Not imported (AboutPage)

Fixes #4896
Also Fixes #4895

@netlify
Copy link

netlify bot commented Mar 28, 2022 •
edited
Loading

✅ Deploy Preview for redwoodjs-docs ready!

Name Link
🔨 Latest commit 49670f9
🔍 Latest deploy log https://app.netlify.com/sites/redwoodjs-docs/deploys/624886a253dda10009f38411
😎 Deploy Preview https://deploy-preview-4948--redwoodjs-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@dac09 dac09 added the release:fix This PR is a fix label Mar 28, 2022
@dac09 dac09 marked this pull request as ready for review March 29, 2022 09:13
dac09 added 3 commits March 29, 2022 16:21
@dac09
Reduce unecessary files from fixture
18ac6ba
@dac09
Merge branch 'main' of github.com:redwoodjs/redwood into fix/subdirec…
82641ef 
…tory-routing

* 'main' of github.com:redwoodjs/redwood:
  chore(deps): update dependency cypress to v9.5.3 (redwoodjs#4955)
  chore(deps): update dependency @supabase/supabase-js to v1.33.1 (redwoodjs#4951)
  chore(deps): update dependency @clerk/types to v2 (redwoodjs#4920)
  fix flightcontrol template and doc (redwoodjs#4934)
  Fix test-project linting errors (redwoodjs#4942)
  More clerk doc comment updates (redwoodjs#4943)
  fix(deps): update dependency eslint to v8.12.0 (redwoodjs#4938)
  chore(deps): update dependency esbuild to v0.14.28 (redwoodjs#4936)
  Enable vscode debugging on api server (redwoodjs#4904)
@dac09
Merge branch 'main' into fix/subdirectory-routing
cbdafb5
@dac09 dac09 added this to the next-release-patch milestone Mar 29, 2022
@dac09 dac09 requested a review from Tobbe March 29, 2022 16:30
Tobbe
Tobbe reviewed Mar 29, 2022
View reviewed changes
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.

A question and a comment for now. Will continue my review tomorrow

tasks/smoke-test/tests/prerender.spec.ts Show resolved Hide resolved
tasks/smoke-test/tests/prerender.spec.ts Outdated Show resolved Hide resolved
dac09 added 3 commits March 30, 2022 13:18
@dac09
Close browser in afterAll
f249a7b
@dac09
Merge branch 'main' of github.com:redwoodjs/redwood into fix/subdirec…
d9cdb2a 
…tory-routing

* 'main' of github.com:redwoodjs/redwood:
  chore(deps): update dependency @types/lodash to v4.14.181 (redwoodjs#4961)
  Add setup step for prisma client on Layer0 provider (redwoodjs#4950)
  fix language and highlighted lines (redwoodjs#4962)
  docs(md): improve code blocks (redwoodjs#4941)
@dac09
Remove browser close, race conditions seem to interfere
a7ed470
@dac09
Copy link
Contributor Author

dac09 commented Mar 31, 2022

@Tobbe also added a fix for #4895 - its a hard one to explain properly, so I've added tests and done explainer for you here: https://s.tape.sh/fL2FKC6e

@dac09
Merge branch 'fix/subdirectory-routing' of github.com:dac09/redwood i…
012ede7 
…nto fix/subdirectory-routing

* 'fix/subdirectory-routing' of github.com:dac09/redwood:
  Clarify the cell aliasing section of tutorial (redwoodjs#4964)
  s/posts/articles (redwoodjs#4971)
  fix(deps): update dependency cross-undici-fetch to v0.1.28 (redwoodjs#4966)
  Add Storybook template for i18n (redwoodjs#4764)
  Add @storybook/addon-essentials by default (redwoodjs#4765)
@dac09 Pullflow.com
Copy link
Contributor Author

dac09 commented Apr 1, 2022

@Tobbe ok to merge or do you want another pass?

@Tobbe Pullflow.com
Copy link
Member

Tobbe commented Apr 1, 2022

I'm finishing up my day job stuff right now. I'll take a look in ~30 min

dac09 added 2 commits April 3, 2022 00:23
@dac09
Undo bad copy
d2d09a0
@dac09
Merge branch 'main' of github.com:redwoodjs/redwood into fix/subdirec…
49670f9 
…tory-routing

* 'main' of github.com:redwoodjs/redwood:
  Update comment-form.md (redwoodjs#4990)
  update details css (redwoodjs#4991)
  Fix issues in rbac.md (redwoodjs#4989)
  fix(deps): update storybook monorepo to v6.4.20 (redwoodjs#4979)
  fix(deps): update dependency @graphql-tools/schema to v8.3.6 (redwoodjs#4977)
  Reduce spacing (redwoodjs#4976)
  add 10 contribs
  Make docs styling a little more consistent (redwoodjs#4981)
  docs: Rename "Serverless.com" menu (redwoodjs#4975)
  Change block quote colours to make it standout more (redwoodjs#4925)
  fix(deps): update docusaurus monorepo to v2.0.0-beta.18 (redwoodjs#4928)
@Tobbe Tobbe enabled auto-merge (squash) April 2, 2022 17:32
@Tobbe Tobbe merged commit d90da07 into redwoodjs:main Apr 2, 2022
@thedavidprice
Copy link
Contributor

Just so you know, adding this one to the final final ultimate RC patch terrifies me.

BUT

I trust you gents 😉

Also YOLO 🤘

thedavidprice pushed a commit that referenced this pull request Apr 2, 2022
@dac09 @thedavidprice
fix(routes): Handle explicitly imported pages with different names (#…
91b98c8 
…4948)
@jtoar jtoar mentioned this pull request Mar 21, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Tobbe Tobbe Tobbe left review comments

Assignees

@dac09 dac09

Labels
release:fix This PR is a fix
Projects
No open projects
Release
Status: Archived
Milestone
v1.0.0-rc.final.4
3 participants
@dac09 @Tobbe @thedavidprice