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

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

Open
neotrow opened this issue Feb 15, 2024 · 6 comments
Open

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

neotrow opened this issue Feb 15, 2024 · 6 comments

Comments

@neotrow
Copy link

neotrow commented Feb 15, 2024

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:
#136

@tatethurston
Copy link
Owner

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
Copy link

decrease the incidence rate of merge conflicts

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

@tatethurston
Copy link
Owner

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
Copy link
Author

neotrow commented Mar 14, 2024

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
Copy link
Owner

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.

tatethurston added a commit that referenced this issue Oct 9, 2024
tatethurston added a commit that referenced this issue Oct 9, 2024
@tatethurston
Copy link
Owner

I published 2.2.4-rc.1 where cli invocation eg: npx nextjs-routes, will read in next.config

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants