search

tatethurston / nextjs-routes

Type safe routing for Next.js
MIT License
571 stars 23 forks source link

doesn't work for `src/pages` directory #9

Closed sachinraja closed 2 years ago

sachinraja commented 2 years ago

Hey, thanks for the amazing project! In my app I use the src/pages directory but it doesn't work for nextjs-routes because it only looks in the pages directory.

tatethurston commented 2 years ago

Hey @sachinraja thanks for reporting this. I just published v0.0.9 to enable this -- let me know if you run into any issues or have other ideas for improvement!

sachinraja commented 2 years ago

Wow that was fast! Works great now. One idea - it would be great to have an ESM build because the CJS build generates a lot of cruft. Will definitely let you know if I have any more ideas, but it's been very nice to work with so far.

tatethurston commented 2 years ago

@sachinraja Thanks for pointing that out -- that was a silly oversight on my end. Now publishing ESM in v0.0.10

sachinraja commented 2 years ago

Thanks for the responsiveness! The implementation is actually not ESM compliant though. ESM files must have the .mjs extension. It might be difficult to configure this using Babel, so I recommend using https://github.com/privatenumber/pkgroll or https://github.com/egoist/tsup (zero-config solutions!) for building dist. I can do a PR if you're ok with that.

tatethurston commented 2 years ago

@sachinraja the mjs extension is a proposal but not a requirement for consuming ESM. AFAIK the only use case is in Node.js, I’m not aware of many client libraries that ship .mjs. If you use a bundler like webpack, roll up or vite the ESM export will be picked up out of the box.

sachinraja commented 2 years ago

The other libraries are incorrect. The bundlers are also incorrect but are forgiving because libraries are usually incorrect. It will break in cases like testing, where files are not bundled and just run through Node.

tatethurston commented 2 years ago

Nextjs’ default webpack and jest config will correctly handle ESM without the mjs extension so is there a particular outcome you’re unable to achieve here?

nodejs will also handle ESM without the mjs extension when the package json type is set to module, so I think the only use case is mixed nodejs projects (consuming commonjs2 and ESM), so I think the assertion that the other libraries and bundlers are incorrect, is overly strong.

sachinraja commented 2 years ago

nodejs will also handle ESM without the mjs extension when the package json type is set to module

This is on a per-package basis. So "type": "module" would have to be set in this project for that to work. Anyway, it's alright.

tatethurston commented 2 years ago

@sachinraja Ah, you’re right I see what you’re saying. Is there a reason to prefer publishing .mjs over setting type: module for this package (other than support for nodejs runtimes that predate that package.json key)?

sachinraja commented 2 years ago

Well if you set type:module you'll have to use .cjs extensions for commonjs. See https://nodejs.org/api/packages.html#determining-module-system

tatethurston commented 2 years ago

@sachinraja closing the loop on this, this library now ships ESM exclusively. I did some testing in the nextjs examples and couldn't find a need for cjs, but if you run into one LMK and I'll add a cjs export.

sachinraja commented 2 years ago

Thanks! I'll let you know if we run into any issues.