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

refactor(useFilenamingConvention): support dynamic route filenames #3466

Merged
merged 1 commit into from
Jul 20, 2024

Conversation

Conaclos
Copy link
Member

@Conaclos Conaclos commented Jul 18, 2024

Summary

Fix #3465

I had to modify the way we generate test names. To have a more robust approach, we now rely on biome_string_case case conversion. I had to change some test name to accommodate with this change.

Test Plan

I added four tests.

@github-actions github-actions bot added A-Linter Area: linter A-Parser Area: parser L-JavaScript Language: JavaScript and super languages L-JSON Language: JSON and super languages A-Changelog Area: changelog labels Jul 18, 2024
Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Any chance we can support SolidStart too? They use a different approach... 😓

https://docs.solidjs.com/solid-start/building-your-application/routing#renaming-index

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link

codspeed-hq bot commented Jul 18, 2024

CodSpeed Performance Report

Merging #3466 will not alter performance

Comparing conaclos/useFilenamingConvention-3465 (46f6378) with main (ff6a9a1)

Summary

✅ 104 untouched benchmarks

@Conaclos
Copy link
Member Author

Conaclos commented Jul 18, 2024

Any chance we can support SolidStart too? They use a different approach..

I will take a look.
All these syntaxes are so ugly to my eyes.

@ematipico
Copy link
Member

All these syntaxes are so ugly to my eyes.

Ehehe, you're aren't wrong 😅

@Conaclos Conaclos force-pushed the conaclos/useFilenamingConvention-3465 branch from 4075f62 to e89cd00 Compare July 18, 2024 16:49
@Conaclos Conaclos requested a review from ematipico July 18, 2024 16:49
@minht11
Copy link
Contributor

minht11 commented Jul 18, 2024

Just to add svelte kit also uses +page@(app).svelte to escape layout https://kit.svelte.dev/docs/advanced-routing#advanced-layouts-page

@Conaclos
Copy link
Member Author

Conaclos commented Jul 18, 2024

+page@(

Is (app) a placeholder?

Just to add svelte kit also uses +page@(app).svelte to escape layout https://kit.svelte.dev/docs/advanced-routing#advanced-layouts-page

Could you detail the syntax?

  • Is page/layout fixed strings? or is it the name of the page/layout?
  • If I understand corerctly, the part behind @ is optional?
    Are (app)/[id] placeholders or are app/id placeholders?

@minht11
Copy link
Contributor

minht11 commented Jul 18, 2024

Anything after @ is just regular route name, it tells from which route above it should inherit nested layout. +page@(app) just means that layout should break out to (app) above. @ is optional and advanced feature.

I also think this should be supported only for svelte files since it is quite unusual syntax. I am not 100% garunateed, but I can't find anything for non svelte files on search. svelte files using that pattern for reference.
image

@Conaclos Conaclos force-pushed the conaclos/useFilenamingConvention-3465 branch from e89cd00 to 46f6378 Compare July 18, 2024 18:36
@Conaclos
Copy link
Member Author

I am going to merge this PR. I will consider extending the rule with a configuration to customize the accepted patterns.

@Conaclos Conaclos merged commit 84384cf into main Jul 20, 2024
15 checks passed
@Conaclos Conaclos deleted the conaclos/useFilenamingConvention-3465 branch July 20, 2024 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Changelog Area: changelog A-Linter Area: linter A-Parser Area: parser L-JavaScript Language: JavaScript and super languages L-JSON Language: JSON and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

useFilenamingConvention and file based routing catch all [...slug]
3 participants