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

[Filesystem Routing] Avoid samed named files (e.g. 10 filenames +Page.tsx) #1322

Open
dfrkp opened this issue Dec 6, 2023 · 13 comments
Open
Labels
enhancement ✨ New feature or request

Comments

@dfrkp
Copy link

dfrkp commented Dec 6, 2023

Description

Hi 👋 ! First of all congrats on reaching v1!

I'm unfortunately though holding back on upgrading to the "v1 design" because of the weird way file system routing is behaving now.

To me, the file name of a source code file is much more significant than the folder it is in. I also believe that there are quite a few tools that work better when different files have different names.

So I'm hoping there will be a possibility to retain logical filenames when using filesystem-based routing. If the files need to be in separate folders, fine, but I should be able to name them in whichever way I feel is best.

@dfrkp dfrkp added the enhancement ✨ New feature or request label Dec 6, 2023
@brillout brillout changed the title Filesystem routing with filenames other than +Page.tsx [Filesystem Routing] Avoid samed named files (e.g. 10 filenames +Page.tsx) Dec 6, 2023
@brillout
Copy link
Member

brillout commented Dec 6, 2023

How about:

# Route: /about
/pages/about+Page.js
# Route: /products
/pages/products+Page.js
# Route: /product/@id
/pages/product/@id+Page.js

Looks weird but it's functional.

Upvote and/or comment down below if you want/need this.

@brillout brillout mentioned this issue Dec 6, 2023
@dfrkp
Copy link
Author

dfrkp commented Dec 6, 2023

That would definitely work for me (it isn't working right now though according to my testing, right?)

@brillout
Copy link
Member

brillout commented Dec 6, 2023

Correct, this isn't implemented yet. I'm busy with high priorities at the moment, but let's see if I can take a stab at it sometime.

There is also #341.

@dfrkp Btw. would your company be up for sponsoring?

@redbar0n
Copy link
Contributor

Instead of:

# Route: /about
/pages/about+Page.js
# Route: /products
/pages/products+Page.js
# Route: /product/@id
/pages/product/@id+Page.js

How about:

# Route: /about
/pages/+about.page.js
# Route: /products
/pages/+products.page.js
# Route: /product/@id
/pages/product/+@id.page.js

Or even just:

# Route: /about
/pages/+about.js
# Route: /products
/pages/+products.js
# Route: /product/@id
/pages/product/+@id.js

Re my earlier comment: #578 (comment)

The arguments for and against + for +Page.ts also go for +onBeforeRender.ts and +route.ts which are also in the V1 design:
https://vike.dev/migration/v1-design

Some arguments in favor of the + prefix, specifically sortability and grouping of Vike related files: #578 (comment)

@brillout
Copy link
Member

@redbar0n Neat ideas, I like them. Although it's a bit unconventional to use the file extension to denote semantics. (That said the whole + thing is unconventional to begin with 😅.)

I think we're going in the right direction 👀

So far I like misusing the file extensions as you propose. Actually, I'm just realizing that Telefunc does that already with its .telefunc.js files.

Actually, there is an issue currently when using Vike with Telefunc:

# Currently, users define this: (confusing as +Page.telefunc.js isn't a Vike file)
/pages/some-page/+Page.js
/pages/some-page/+Page.telefunc.js
# Or this: (ugly)
/pages/some-page/+Page.js
/pages/some-page/Page.telefunc.js

Maybe we can solve that issue as well while we're at it.

@brillout
Copy link
Member

Maybe the trick is to have Vike support both: the + syntax as of today, as well as the suffix syntax:

/pages/product/@id.Page.js
/pages/product/@id.route.js
/pages/product/@id.telefunc.js

(I understand that Page starting with a capital P is offsetting at first, but I think users get used to it quickly so I don't think it's much of an issue. It's important to have a consistent capitalization: @id.Layout.js, @id.Page.js, +config.js > export default { Page, Layout }. Alternatively, maybe we could lower-case all component setting names thus breaking the convention that component functions start with a capital letter 🤔)

@brillout
Copy link
Member

Those who prefer having page files tucked in their own directly could use + (like what's currently already possible):

/pages/product/@id/+Page.js
/pages/product/@id/+route.js
# We can make Telefunc understand that syntax
/pages/product/@id/+telefunc.js

In other words /pages/product/@id/+Page.js is the same as and just a prettier version of /pages/product/@id/index.Page.js.

@brillout
Copy link
Member

@redbar0n WDYT?

@tbscode
Copy link

tbscode commented Mar 22, 2024

Hey wanted to add my 2 cents here:

  1. having + in the front is nice as in alphabetical order all the config files are always at the top ( would be lost with Name+Page.tsx )
  2. but it's indeed annoying that it's not possible to specify a custom name for a page ( make fuzzy search and filtering quite annoying )

To me allowing something like +Page.SomeName.tsx would sound quite nice.

@redbar0n
Copy link
Contributor

redbar0n commented Mar 25, 2024

@brillout Cool! I'll try to summarize my thoughts and give a suggestion.

I realized the part of my suggestions using /pages/product/+@id.page.js or /pages/product/+@id.js doesn't actually solve the problem raised by the original post, since those file names would be repeated everywhere. Such file names are not clear and self-describing in isolation.

So how about this?

I think the following would be ideal, imho. Prioritising maximum clarity (even without path info available) over terseness, and minimum necessary learning / documentation / variability:

# flat
/pages/about.page.js
...
/pages/about.telefunc.js

#folder
## the index
/pages/products/products.config.js
/pages/products/products.layout.js
/pages/products/products.onBeforeRender.js
/pages/products/products.page.js
/pages/products/products.route.js
...
/pages/products/products.telefunc.js

## an item
/pages/product/@productId.config.js
/pages/product/@productId.layout.js
/pages/product/@productId.onBeforeRender.js
/pages/product/@productId.page.js
/pages/product/@productId.route.js
...
/pages/product/@productId.telefunc.js

# similarly, for the generic error page, instead of /pages/_error/+Page.js you could have:
/pages/error.page.js

# similarly for general configs
/pages/pages.config.js # general config
# or, maybe this is better, since people would likely fuzzy search/filter for "vike config" for everything they'd expect Vike to handle
/pages/vike.config.js # general config, named similarly to other frameworks: remix.config.js 

# individual custom configs overriding the general one
/pages/admin/admin.config.js
/pages/product/@productId.config.js # like illustrated initially above

This should also obviate the need to rename files when moving them from a flat structure into a folder.

I dropped the + since a page isn't really a config file. For uniformity, I dropped the + even for the config files. It also resolves the question of convention when combined with telefunc.

I dropped having a .vike extension too (e.g. products.onBeforeRender.vike.js), to at least achieve some brevity. Otherwise, all 4 of the vike related files should ideally have that extension, but since Vike is a framework and a framework owns the whole app, then it's more natural to just consider it the general context and drop the extensions for that. The user would have to learn from docs that those 4 files belong to Vike, instead of being able to always infer it from the file names, but that seems a reasonable tradeoff, esp. since those types of files will be repeated everywhere. But most people would likely conclude with the rule that "Files with unusual file name extensions are used by the framework". For libraries like Telefunc, on the other hand, it's more important to use a file name extension to clarify which files are specifically used by that library, so .telefunc.js.

Vike ideally only has one single syntax: less variation means less opportunity for user error, less documentation needed, less to learn, and easier to go into a new Vike codebase because everything will be familiar. I'd go with the suffix syntax because it is clearer and self-explanatory. If it is really important to show config files at the top of the folder, then people can add + or - or some other prefix if they wish. But if config files are not modified that often, I'd rather have them at the bottom really, out of the way. (From the framework's perspective then its own config is most important, but from the app developer perspective his/her app files are more important.) Just goes to show that "what should be the focus of attention" is very subjective, and shouldn't be dictated by the framework.

Capitalizing Page because it is a component would seem to base the name on semantics. Imho, it's also unintuitive to apply this component convention outside of the JS files - they are just JS files. My conceptualization is that the convention to capitalize component names are something done within the code because of a React / JSX convention. Whereas I consider the file system to belong to the framework, which can have its own conventions irrespective of the chosen view library (esp. when agnostic to it). Uniformity / consistency ("Vike always lowercases the start of file names") also raises fewer questions. By avoiding a one-off convention in the file system, there's less to explain ("Why does Vike capitalize some file names but not others?"), and less to document. The two worlds would meet when you make an:

import { AboutPage } from "/pages/about/about.page.js"

What do you guys think?

@redbar0n
Copy link
Contributor

Or just, like previously suggested (which seemed popular), collate all the Vike stuff into one explicity declared file, so you'd have:

#folder
## the index
/pages/products/products.page.js
/pages/products/products.vike.js
...
/pages/products/products.telefunc.js

## an item
/pages/product/@productId.page.js
/pages/product/@productId.vike.js
...
/pages/product/@productId.telefunc.js

I think maybe I prefer this, since the amount of files in the file system directory structure often quickly grows out of hand to actually have a nice overview. It also make having a flat folder structure more viable, as the amount of Vike related files would be greatly minimized.

@redbar0n
Copy link
Contributor

redbar0n commented Mar 25, 2024

If you really want to keep the capitalized component name convention, then...

Instead of:

/pages/products/products.page.js

Then the convention could be:

/pages/products/ProductsPage.js

which would make it clearer that ProductsPage is a component like all the other components in the folder capitalized that way.

Then importing would be like:

import { ProductsPage } from "/pages/products/ProductsPage.js"

@redbar0n
Copy link
Contributor

redbar0n commented Mar 25, 2024

Not sure if it makes sense, but you could rename the Vike file too, for symmetry / aesthetical reasons. Conceptually you'd think of it as "Vike configures a page":

# the index
/pages/products/ProductsPage.js
/pages/products/ProductsPage.vike.js

# item
/pages/product/@ProductIdPage.js
/pages/product/@ProductIdPage.vike.js

Even better / clearer perhaps:

# the index
/pages/products/Products.js
/pages/products/Products.vike.js

# item
/pages/product/@ProductId.js
/pages/product/@ProductId.vike.js

Where by convention the file with the same name as the .vike file in the same folder would be treated as the index/page. Relying on the .vike file name and not the folder name for the convention. That way, you don't even have to rename files when moving them out/into folders (e.g. /pages/Products.js to /pages/products/Products.js). You'd also enforce the conception that "Vike can treat any component as a page". Not sure if that would open up any opportunnities, but maybe less renaming if refactoring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ✨ New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants