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

kit-routes: add exportConsts config option #775

Merged

Conversation

gwennlbh
Copy link
Contributor

@gwennlbh gwennlbh commented Nov 22, 2024

Hi!

We're using kit-routes in our app and it works great!

However, we have users that can choose the username, and get the profile page on /username. Thus, we want to disallow usernames that conflict with existing routes, so that their profile page is not inaccessible because they chose e.g. "settings" as a username

kit-routes actually generates the strings we need for this! unfortunately, they're not exported...

right now I'm using a yarn patch to get them, but having it in the config will certainly be better ^^

So this PR adds the exportConsts option that decides whether to export PAGES, etc. or not

net7toulouse pushed a commit to inp-net/churros that referenced this pull request Nov 23, 2024
…ed-dotenv-service-worker

- for reserved usernames, we modify the lib/ROUTES.ts that's kept up to
date by kit-routes, exporting the various consts it generates (only the
types are exported, and we need runtime checks here...). I patched
vite-plugin-kit-routes right now, we'll wait to see if
jycouet/kitql#775 is merged
Copy link

nx-cloud bot commented Nov 23, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 50a56e3. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@jycouet
Copy link
Owner

jycouet commented Nov 23, 2024

@all-contributors please add @ewen-lbh for code

Copy link
Contributor

@jycouet

I've put up a pull request to add @ewen-lbh! 🎉

@jycouet jycouet merged commit 64415c5 into jycouet:main Nov 23, 2024
3 checks passed
@jycouet
Copy link
Owner

jycouet commented Nov 23, 2024

🙏 thx for the contribution.
Sorry, I messed up with the flow of merge / commit / contrib / ... But it should be good now.

Can you try [email protected] and let me know if it's ok for you?

@gwennlbh
Copy link
Contributor Author

just upgraded, it works fine ^^

also sorry, i just realized, i probably should've named the option in snake_case rather than camelCase... oh well

@jycouet
Copy link
Owner

jycouet commented Nov 23, 2024

Well well, your touch is in at least ;)
FYI: https://bsky.app/profile/jyc.dev/post/3lbm3mi5rok23
🙏 Merci bien :)

net7toulouse pushed a commit to inp-net/churros that referenced this pull request Nov 23, 2024
@gwennlbh
Copy link
Contributor Author

omg

about to setup a AT protocol for my own domain just to reply lmao (also i really gotta update my github username)

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

Successfully merging this pull request may close these issues.

2 participants