Skip to content

Conversation

@I-3B
Copy link
Contributor

@I-3B I-3B commented Oct 25, 2025

closes #1022 #1036 #1118

Checklist

  • Registry package
  • parseAsTuple registry item
  • unit tests for parseAsTuple
  • add usage .md for parseAsTuple

@vercel
Copy link

vercel bot commented Oct 25, 2025

@I-3B is attempting to deploy a commit to the 47ng Team on Vercel.

A member of the Team first needs to authorize it.

@I-3B I-3B marked this pull request as draft October 25, 2025 12:12
@franky47
Copy link
Member

Thanks! I was working on the docs part (listing of installable items and copy/paste vs CLI methods) while on the plane, I'll open a PR later today.

@I-3B
Copy link
Contributor Author

I-3B commented Oct 25, 2025

hi @franky47 !
this is still WIP, but would like your opinion on exposing lib/ like this
also I'm not entirely sure about the trusted registry part, do we need to submit anything to shadcn?

@franky47
Copy link
Member

franky47 commented Oct 25, 2025

Yeah I'll open a PR to the shadcn registry, as we have the green light from the man himself, but once #1189 is live.

I have opened #1189 as a draft too for the docs UI part, we could stack yours on top of that one to add parseAsTuple.

At a first glance, I'm not super keen on widening the exported API for internal purposes, I'll have to take a closer look once I'm home. But in #1189 I have all the "sources" for the registry as part of where it is deployed (in the docs package), and that causes problems in terms of dependencies, so a separate package might make more sense.

Like I wanted to add the equivalent of the next-typed-links snippet for React Router's href utility, but that would have implied adding the react-router dependency to the docs, along with tests harnesses etc.. Better do a separate package, have it build the registry JSON files and mark it as a dependency of the docs. We'll still need to copy the built output into the docs' public/r directory (having root level item URLs might clash and the trusted registry mapping makes the /r/ subpath invisible anyway) before build starts (as the registry docs page read those files to populate its content).

@I-3B
Copy link
Contributor Author

I-3B commented Oct 25, 2025

At a first glance, I'm not super keen on widening the exported API for internal purposes

agree, alternative is to add a lib/utils.ts (like shdacn's cn function), but didn't go that way cause it will mean there will be two copies of this util to maintain (which i think is okay, it's not a big function anyway)

@I-3B I-3B changed the title Shadcn Registry Package feat: Shadcn Registry Package Oct 25, 2025
Copy link
Contributor Author

@I-3B I-3B Oct 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't add debug file as I thought it's an internal package detail that shouldn't be put in users' files, tradeoff of course is that debug flag doesn't work with community parsers

@I-3B I-3B marked this pull request as ready for review October 26, 2025 11:23
Copy link
Member

@franky47 franky47 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I merged #1189, feel free to rebase on top of that.

It might make sense to expose the safeParse function as initially suggested (which would also make the feat: prefix a legitimate minor bump for the nuqs package).

Comment on lines 14 to 16
"path": "parsers/parseAsTuple.ts",
"type": "registry:file",
"target": "~/components/parsers/parseAsTuple.ts"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: This would also need to install the safeParse utility as it's used in the parseAsTuple file.

Maybe it would make sense to export it from nuqs then, as it's useful for custom parsers anyway.

@franky47 franky47 added the registry Issues & PRs related to the shadcn registry label Oct 26, 2025
@franky47 franky47 added this to the 🪵 Backlog milestone Oct 26, 2025
@vercel
Copy link

vercel bot commented Oct 28, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
nuqs Error Error Dec 7, 2025 1:27pm

Copy link
Member

@franky47 franky47 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, there has been a few changes in how the registry is built in #1209:

  • Each item is now described in an individual packages/docs/src/registry/{item-name}.json
  • Next to that file can be an optional {item-name}.md for usage docs
  • Those items can specify a URL for file paths, in which case its contents will be fetched when building the registry (we use this for the Inertia adapter which lives in a PHP example monorepo).

I think we could modify the "assemble" script that pulls all these items together into a generated root registry.json file to also go read from packages/docs/node_modules/registry/items/**/*.json (symlinked to packages/registry/items/**/*.json) or something similar, to enforce good boundaries between packages: registry for testing items, docs for deployment.

I thing there would still be cases where items could live:

  • In the docs directly, if simple enough, or if an integration with Next.js is needed (eg: typed links)
  • Outside the repo, if the test harness pulls too many dependencies that would slow down CI (eg: adapters for One.js or Expo Router, we might not want to add a full React Native stack to this monorepo just for one copy-pastable block of code).

But there is a lot of value in having items living in a testable registry package (or set of packages if we see in the future that some test harnesses conflict with one another).

I'm going to be preparing for a talk at React Advanced at the end of the week, so I might not be super active in PRs, but if you need guidance in how to adapt this PR to fit that model, feel free to ping me here. I've left some comments here and there too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: This one would be difficult to test without having a proper Next.js setup to typegen route definitions from, which we do have in the docs and could test against there.

@I-3B
Copy link
Contributor Author

I-3B commented Nov 23, 2025

Thanks for the review and good luck with the talk!

In the docs directly, if simple enough, or if an integration with Next.js is needed (eg: typed links)

even if the item is simple enough, we still would want to have it tested, yes? (e.g. parseAsTuple), so either we add tests right in docs package, or move all items, even simple ones, to registry package as they need to be tested.

My worry is that it will be hard to maintain 2 resources of registry items with no clear distinction on why a particular item is under docs or registry.

so need to decide if we want to discard the registry package altogether, and keep everything under docs (with their tests), or move all items to be under registry package, and update assemble script to pull from registry package.

or something else?

@franky47
Copy link
Member

I was thinking of the nextjs-typed-routes, which pulls type definitions from the next package, which are generated. This one could leverage the setup in the docs, or we could replicate a test setup in the registry package.

@I-3B
Copy link
Contributor Author

I-3B commented Nov 23, 2025

I see, okay then as a start I'll update the registry to work with the assemble script, might also move the current registry items that are under docs to be under registry (except nextjs-typed-routes).

@I-3B I-3B requested a review from franky47 December 6, 2025 19:41
@I-3B I-3B force-pushed the feat/registry branch 3 times, most recently from 2dfd858 to 081f9ed Compare December 6, 2025 19:50
Copy link
Member

@franky47 franky47 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the registry dependency looks much better. However, it looks like the usage markdown file wasn't picked up:

Image

Edit: yeah the readUsage function still expects it to be located in the docs registry directory:
https://github.com/47ng/nuqs/blob/next/packages/docs/src/registry/read.ts#L35-L36

parseAsTuple([parseAsInteger, parseAsInteger])

// Optionally, customise the separator
parseAsTuple([parseAsInteger, parseAsInteger], ';')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: we could showcase a combination of different parsers in those docs, like [string, 'asc' | 'desc'] for a sorting tuple or something similar.

@I-3B I-3B requested a review from franky47 December 7, 2025 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

registry Issues & PRs related to the shadcn registry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

parseAsTuple

2 participants