refactor: use routes constant for all internal paths #1071

evgenyboxer · 2022-10-21T06:16:59Z

Hopefully I've covered all instances of the internal paths! I've replaced them all with the routes constant.

vercel · 2022-10-21T06:17:02Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
gallery	✅ Ready (Inspect)	Visit Preview	Oct 26, 2022 at 2:02PM (UTC)

src/scenes/LandingPage/LandingPage.tsx

pages/[username]/[collectionId]/[tokenId]/index.tsx

src/constants/routes.ts

src/scenes/LandingPage/LandingPage.tsx

Robinnnnn

based

tbezman · 2022-10-21T13:30:07Z

@evgenyboxer Have you looked into this? https://github.com/ckastbjerg/next-type-safe-routes

Seems like it could get us most of the benefits without any of the maintenance.

Also would get us type safety on params and stuff.

Definitely curious to hear your thoughts. Let me know!

evgenyboxer · 2022-10-22T11:05:45Z

Hey @tbezman

That's absolutely the way to go, had no idea this existed. My only concern with this specific library is that it doesn't look widely used / maintained.

I had a spin on it, and found a couple of issues -

It seems that they require an api folder to be present inside the pages (took me a little while to debug to reach this conclusion) and this is reproduced in their example repo as well. Workaround is to create an empty api folder and then it would generate the types. However, it generates them with TypeSafeApiRoute which is essentially assigned to any and that kinda breaks the type safety if you directly use their getRoute (caused by the empty api folder). I find it odd that they require an api folder... but anyways, if they would at least not generate TypeSafeApiRoute it would work well in this case. We could create a new getSafeRoute that just uses the TypeSafePage type without TypeSafeApiRoute.
When using with Next.js v12 there are some warnings coming out of the next.js config, but still it generates the types fine.
You can't change the location of where the types are generated, seems there is a PR opened from March but wasn't merged - Configurable output directory ckastbjerg/next-type-safe-routes#35

Happy for you to play with this as well and let me know if you find success in using it.

I really like the direction of this library, it is definitely an improvement over my solution.

…d-routes-constant

evgenyboxer · 2022-10-24T11:42:58Z

next.config.js


-const moduleExports = {
+/** @type {import('next').NextConfig} */


This gives a nice autocomplete for next config options

evgenyboxer · 2022-10-24T11:43:18Z

next.config.js

-module.exports = withSentryConfig(moduleExports, sentryWebpackPluginOptions);
+const plugins = [withRoutes, (config) => withSentryConfig(config, sentryWebpackPluginOptions)];
+
+module.exports = () => plugins.reduce((config, plugin) => plugin(config), nextConfig);


Followed the thread here - cyrilwanner/next-compose-plugins#59

evgenyboxer · 2022-10-24T11:44:58Z

pages/_/components.tsx

-          <ButtonLink href="#">primary</ButtonLink>
-          <ButtonLink href="#" pending>
+          <ButtonLink href={{ pathname: '/' }}>primary</ButtonLink>
+          <ButtonLink href={{ pathname: '/' }} pending>


@tbezman Would that work? or do we need a workaround here?

No problem here

evgenyboxer · 2022-10-24T11:47:18Z

src/components/Feed/Socialize/AdmireLine.tsx

+    ? { pathname: '/[username]', query: { username: admire.admirer.username } }
+    : { pathname: '/' };
+
+  const admirerLink = route(admirerLinkRoute);


I'm using this pattern here since <Link wants a Route and AdmirerName wants a string.

Makes sense to me. Next 13 removes this double tag nonsense so we'll be able to just use the route!

evgenyboxer · 2022-10-24T11:55:46Z

src/types/Elements.ts

+
+export type InternalAnchorElementProps = Omit<AnchorElementProps, 'href'> & {
+  href: Route;
+};


@tbezman I wasn't sure of a good name for this, so went with something generic (Elements.ts) that could house similar types in the future.

evgenyboxer · 2022-10-24T12:07:37Z

next.config.js


-const moduleExports = {
+/** @type {import('next').NextConfig} */


This gives us a new autocomplete for the config

refactor: use routes constant for all internal paths

3f81871

vercel bot deployed to Preview October 21, 2022 06:18 View deployment

evgenyboxer commented Oct 21, 2022

View reviewed changes

src/scenes/LandingPage/LandingPage.tsx Outdated Show resolved Hide resolved

evgenyboxer changed the title ~~[wip]: refactor: use routes constant for all internal paths~~ refactor: use routes constant for all internal paths Oct 21, 2022

evgenyboxer requested review from Robinnnnn, jakzaizzat and tbezman October 21, 2022 06:19

fix: remove string casting

cbe2118

vercel bot deployed to Preview October 21, 2022 06:25 View deployment

fix: TS issues

71d931c

Robinnnnn reviewed Oct 21, 2022

View reviewed changes

pages/[username]/[collectionId]/[tokenId]/index.tsx Outdated Show resolved Hide resolved

Robinnnnn reviewed Oct 21, 2022

View reviewed changes

src/constants/routes.ts Outdated Show resolved Hide resolved

Robinnnnn reviewed Oct 21, 2022

View reviewed changes

src/scenes/LandingPage/LandingPage.tsx Outdated Show resolved Hide resolved

vercel bot deployed to Preview October 21, 2022 06:34 View deployment

Robinnnnn approved these changes Oct 21, 2022

View reviewed changes

refactor: use nextjs-routes

f645f2b

vercel bot deployed to Preview October 23, 2022 12:03 View deployment

fix: TS errors

06e1487

vercel bot deployed to Preview October 24, 2022 10:55 View deployment

evgenyboxer added 3 commits October 24, 2022 22:09

fix: add @ts-expect-error in a couple of tricky places

86b0fa6

extract shared anchor element types

63f3b3b

Merge branch 'main' of github.com:gallery-so/gallery into refactor/ad…

ac2466d

…d-routes-constant

vercel bot deployed to Preview October 24, 2022 11:29 View deployment

fix: home path

c71c29e

vercel bot deployed to Preview October 24, 2022 11:38 View deployment

fix: nextjs config warnings

f7db9e8

evgenyboxer commented Oct 24, 2022

View reviewed changes

vercel bot deployed to Preview October 24, 2022 11:45 View deployment

evgenyboxer commented Oct 24, 2022

View reviewed changes

fix unit tests

04b40fc

vercel bot deployed to Preview October 26, 2022 13:46 View deployment

ignore generated routes file

297438b

vercel bot deployed to Preview October 26, 2022 13:55 View deployment

run nextjs-routes when typechecking

a023027

vercel bot deployed to Preview October 26, 2022 14:02 View deployment

tbezman merged commit dbd69ce into main Oct 26, 2022

tbezman deleted the refactor/add-routes-constant branch October 26, 2022 14:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: use routes constant for all internal paths #1071

refactor: use routes constant for all internal paths #1071

evgenyboxer commented Oct 21, 2022 •

edited

Loading

vercel bot commented Oct 21, 2022 •

edited

Loading

Robinnnnn left a comment

tbezman commented Oct 21, 2022

evgenyboxer commented Oct 22, 2022 •

edited

Loading

evgenyboxer Oct 24, 2022

tbezman Oct 26, 2022

evgenyboxer Oct 24, 2022

evgenyboxer Oct 24, 2022

tbezman Oct 26, 2022

evgenyboxer Oct 24, 2022

tbezman Oct 26, 2022

evgenyboxer Oct 24, 2022

tbezman Oct 26, 2022

evgenyboxer Oct 24, 2022


		const moduleExports = {
		/** @type {import('next').NextConfig} */

refactor: use routes constant for all internal paths #1071

refactor: use routes constant for all internal paths #1071

Conversation

evgenyboxer commented Oct 21, 2022 • edited Loading

vercel bot commented Oct 21, 2022 • edited Loading

Robinnnnn left a comment

Choose a reason for hiding this comment

tbezman commented Oct 21, 2022

evgenyboxer commented Oct 22, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evgenyboxer commented Oct 21, 2022 •

edited

Loading

vercel bot commented Oct 21, 2022 •

edited

Loading

evgenyboxer commented Oct 22, 2022 •

edited

Loading