search

tatethurston / nextjs-routes

Type safe routing for Next.js
MIT License
553 stars 20 forks source link

nextjs-routes.d.ts not generated on build #182

Open neotrow opened 5 months ago

neotrow commented 5 months ago

Thanks for the awesome package!

We would like to not check in the nextjs-routes.d.ts file because it leads to merge conflicts quite often.

But than we noticed that the nextjs-routes.d.ts file actually doesn't get created when running a build. This can be reproduced by running yarn build in the examples/typescript project. (And deleting the existing nextjs-routes.d.ts file beforehand)

We also tried to run the cli with yarn nextjs-routes but noticed that this doesn't respect the next.config.js file and hence the build later fails because it doesn't know anything about the locales. But I think that's a known issue: https://github.com/tatethurston/nextjs-routes/issues/136

tatethurston commented 5 months ago

Hey @neotrow. Thanks, there are a few interesting things here:

We would like to not check in the nextjs-routes.d.ts file because it leads to merge conflicts quite often.

Could you tell me more about this? Ideally, providing a few examples? I'm curious if there are some git config recommendations or other opportunities to here decrease the incidence rate of merge conflicts

But than we noticed that the nextjs-routes.d.ts file actually doesn't get created when running a build

Yep, this is intentional because the file is expected to be version controlled. That said, I'm open to supporting a configuration option to generate the file in builds.

yangshun commented 5 months ago

decrease the incidence rate of merge conflicts

Sorting the routes alphabetically should help reduce the chance of conflicts.

tatethurston commented 5 months ago

decrease the incidence rate of merge conflicts

Sorting the routes alphabetically should help reduce the chance of conflicts.

Yeah I was curious about something like that but I'm having trouble reasoning about it. Is that primarily due to fewer same line insertions, or is there something else going on?

neotrow commented 4 months ago

hey @tatethurston sorry for my very late reply!

Could you tell me more about this? Ideally, providing a few examples? I'm curious if there are some git config recommendations or other opportunities to here decrease the incidence rate of merge conflicts

Unfortunatley I can't provide any examples. But in general our approach is to not check in files that can be auto generated as it leads to conflict sooner or later.

Yep, this is intentional because the file is expected to be version controlled. That said, I'm open to supporting a configuration option to generate the file in builds.

I see. One thing to note ist that the Readme says that it is generated on build: https://github.com/tatethurston/nextjs-routes?tab=readme-ov-file#installation--usage- > Start or build your next project:

Would you be open for a PR that will generate the file on build? I haven't checked the code yet but I assume it would be fairly simple to implement? And I guess we could also make it configurable, so one can decide if it should be generated on build or not?

tatethurston commented 4 months ago

I’m happy to take a look at an MR for this. And yes, let’s have this configurable, with the default continuing to be off when building for production.

Separately, I’m also interested in reducing the conflict frequency. I’d like to explore the sorting method that @yangshun called out. If you want to tackle that as well, I’d appreciate it, but totally fine if not.