Remove the indexed type reference on AvailableRoutes #8918

orta · 2023-07-14T21:46:19Z

because referencing a route which doesn't exist causes a runtime error (because they are called fns) - fixes #8908

…ing a route which doesn't exist causes a runtime error (because they are called fns)

dac09

Looks good to me! Thanks for finding all of these little issues Orta

Let's wait for CI to go through

dac09 · 2023-07-28T07:11:37Z

@jtoar I don't think this should be marked as a breaking change - since it'll actually "break to correct you".

If you had a bad route name, it wouldn't complain on build time but fail at run time (which is worse) - this catches those cases.

jtoar · 2023-07-28T07:26:19Z

Yeah happy for it to go in whenever, probably listed it as breaking mostly to remember to include it in v6, but that's out now. I tried to include it for v6, but couldn't get it to pass CI the last time I tried (maybe a few days ago)

dac09 · 2023-07-28T07:28:29Z

@orta looks like with this change a few things don't quite work (trying to index AvailableRoutes). Suggestions for how to fix this?

packages/router/src/AuthenticatedRoute.tsx - it doesn't let us index routes
And in packages/router/src/util.ts - it doesn't let us assign it (for the same reason)

I just added at the types for AuthenticatedRoutes back - we must haven't missed it during the router refactor!

orta · 2023-07-28T07:53:13Z

The generated types might only be available on the web side? But your internal runtime can't know about that, so you'll have to as any those calls most likely, you were effectively doing that already via the indexed type (or you can re-assign the type internally)

dac09 · 2023-07-28T08:02:59Z

or you can re-assign the type internally)

Gave this a try.... I just wish I didn't have to do the as GeneratedRoutesMap reassignment every time. Assigning to a const feels wrong too, because I'm changing actual code just to make TS happy.

Tobbe · 2023-07-28T08:51:29Z

packages/router/src/util.ts

+// Not using AvailableRoutes because the type is generated in the user's project
+// We can't index it correctly in the framework


Suggested change

// Not using AvailableRoutes because the type is generated in the user's project

// We can't index it correctly in the framework

// Not using AvailableRoutes. Because the type is generated in the user's

// project we can't index it correctly in the framework

I don't think the grammar on the suggestion is correct ;) Leaving as is.

Tobbe · 2023-07-28T08:52:15Z

packages/router/src/AuthenticatedRoute.tsx


-export function AuthenticatedRoute(props: any) {
+interface AuthenticatedRouteProps {


Pulled this change out into its own PR if we want to keep this one cleaner #8989

Co-authored-by: Daniel Choudhury <[email protected]>

…nto try/apollo-ssr-stream * 'fix/more-streaming-fixes' of github.com:dac09/redwood: (155 commits) Dont inject a null bundle Rename variable Fix for when a page is explicitly imported Add tests fort portal head fix(deps): update dependency vite to v4.4.8 (redwoodjs#9003) fix(deps): update dependency fast-glob to v3.3.1 (redwoodjs#8997) fix(deps): update storybook monorepo to v7.2.0 (redwoodjs#9002) fix(deps): update prisma monorepo to v5.1.0 (redwoodjs#9001) fix(deps): update dependency fastify to v4.21.0 (redwoodjs#8998) fix(deps): update dependency @vitejs/plugin-react to v4.0.4 (redwoodjs#8999) Rename files, update comments Partly backwards compatible Meta tag injection WIP: Stream injection fix(deps): update dependency @whatwg-node/fetch to v0.9.9 (redwoodjs#8942) v6.0.2 Make sure env var name starts with REDWOOD_ENV_ (redwoodjs#8993) fix(realtime): add misisng `@` in setup command Make sure env var name starts with REDWOOD_ENV_ (redwoodjs#8993) fix(realtime): add misisng `@` in setup command Remove the indexed type reference on AvailableRoutes (redwoodjs#8918) ...

@font-face

…nto try/apollo-ssr-stream * 'fix/more-streaming-fixes' of github.com:dac09/redwood: Whoops Update packages/vite/src/utils.ts More suggestions, fix wrong path on reactRefresh script Use node path for consistency Fix portal head not rendering on the server docs(fonts): Update @font-face recommendation (redwoodjs#8986) Docs: remove useless code in code snippet (redwoodjs#8990) v6.0.3 fix(router): Prevent rerendering authenticated routes on hash change (redwoodjs#9007) Remove the indexed type reference on AvailableRoutes (redwoodjs#8918) Remove debug logs Update packages/vite/src/streamHelpers.ts

@font-face

…croll-reset * 'main' of github.com:redwoodjs/redwood: fix(deps): update dependency pino to v8.15.0 (redwoodjs#9023) fix(deps): update dependency eslint to v8.46.0 (redwoodjs#9022) fix(deps): update dependency react-hook-form to v7.45.4 (redwoodjs#9017) chore(docs): reversion docs to include recent changes fix(deps): update dependency vite to v4.4.9 (redwoodjs#9018) v6.0.4 fix(docs): update quick start to fix Storybook start up (redwoodjs#9014) cherry pick part of "fix: Improve GraphQL Schema Validation to allow Subscription types when not using Realtime and ensure schema does not use reserved names (redwoodjs#9005)" fix: Improve GraphQL Schema Validation to allow Subscription types when not using Realtime and ensure schema does not use reserved names (redwoodjs#9005) Docs: Explain the entry.client.{jsx,tsx} file (redwoodjs#8987) chore(deps): update dependency esbuild to v0.18.19 (redwoodjs#8983) chore(deps): update dependency nx-cloud to v16.2.0 (redwoodjs#8985) docs(fonts): Update @font-face recommendation (redwoodjs#8986) Docs: remove useless code in code snippet (redwoodjs#8990) v6.0.3 fix(router): Prevent rerendering authenticated routes on hash change (redwoodjs#9007) Remove the indexed type reference on AvailableRoutes (redwoodjs#8918)

Remove the indexed type reference on AvailableRoutes because referenc…

ad68825

…ing a route which doesn't exist causes a runtime error (because they are called fns)

orta mentioned this pull request Jul 17, 2023

[RFC]: Simplify route types, and make them easy for people to use #8814

Open

1 task

jtoar added the release:breaking This PR is a breaking change label Jul 17, 2023

jtoar added this to the v6.0.0 milestone Jul 17, 2023

jtoar modified the milestones: v6.0.0, next-release Jul 27, 2023

Merge branch 'main' into tight_routes

931c904

dac09 approved these changes Jul 28, 2023

View reviewed changes

Re-add types we forgot to AuthenticatedRoutes

9d0cab5

jtoar added release:fix This PR is a fix and removed release:breaking This PR is a breaking change labels Jul 28, 2023

Re-assign an internal type for generated routes map

1679035

dac09 added 2 commits July 28, 2023 15:22

Avoid reassignment to const

3e24987

Clean up, DRY it up

b2ac401

Tobbe reviewed Jul 28, 2023

View reviewed changes

Merge branch 'main' into tight_routes

db67b43

dac09 enabled auto-merge (squash) July 28, 2023 08:58

dac09 merged commit 18e131e into redwoodjs:main Jul 28, 2023

jtoar modified the milestones: next-release, next-release-patch, v6.0.2, v6.0.3 Jul 31, 2023

jtoar pushed a commit that referenced this pull request Aug 4, 2023

Remove the indexed type reference on AvailableRoutes (#8918)

fbf9d00

Co-authored-by: Daniel Choudhury <[email protected]>

pvenable mentioned this pull request Aug 9, 2023

Remove Guaranteed from CellSuccessData #9010

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove the indexed type reference on AvailableRoutes #8918

Remove the indexed type reference on AvailableRoutes #8918

orta commented Jul 14, 2023

dac09 left a comment •

edited

Loading

dac09 commented Jul 28, 2023

jtoar commented Jul 28, 2023

dac09 commented Jul 28, 2023 •

edited

Loading

orta commented Jul 28, 2023

dac09 commented Jul 28, 2023 •

edited

Loading

Tobbe Jul 28, 2023

dac09 Jul 28, 2023

Tobbe Jul 28, 2023

		// Not using AvailableRoutes because the type is generated in the user's project
		// We can't index it correctly in the framework


		export function AuthenticatedRoute(props: any) {
		interface AuthenticatedRouteProps {

Remove the indexed type reference on AvailableRoutes #8918

Remove the indexed type reference on AvailableRoutes #8918

Conversation

orta commented Jul 14, 2023

dac09 left a comment • edited Loading

Choose a reason for hiding this comment

dac09 commented Jul 28, 2023

jtoar commented Jul 28, 2023

dac09 commented Jul 28, 2023 • edited Loading

orta commented Jul 28, 2023

dac09 commented Jul 28, 2023 • edited Loading

Tobbe Jul 28, 2023

Choose a reason for hiding this comment

dac09 Jul 28, 2023

Choose a reason for hiding this comment

Tobbe Jul 28, 2023

Choose a reason for hiding this comment

dac09 left a comment •

edited

Loading

dac09 commented Jul 28, 2023 •

edited

Loading

dac09 commented Jul 28, 2023 •

edited

Loading