analogjs / analog

The fullstack meta-framework for Angular. Powered by Vite and Nitro
https://analogjs.org
MIT License
2.5k stars 240 forks source link

Feature: Add support for co-locating TypeScript files next to route files #274

Closed brandonroberts closed 1 year ago

brandonroberts commented 1 year ago

Which scope/s are relevant/related to the feature request?

router

Information

Currently, any file with a .ts extension under thesrc/app/routes folder is treated as a route. If you want to co-locate some small components or TypeScript along with the route it must put outside the routes folder.

This feature would allow you to specify folders within the routes directory that will be ignored for route collection.

Potential folder names

Files included in these folders would be ignored from the file globbing handled by Vite.

Describe any alternatives/workarounds you're currently using

Put components/TypeScript outside the routes folder, such as components, utils, and so on.

I would be willing to submit a PR to fix this issue

brandonroberts commented 1 year ago

cc: @goetzrobin

goetzrobin commented 1 year ago

Thank you @brandonroberts! Do you think we need to give the folder a specific name or could just support excluding any folder that starts with a single underscore? We would probably need to coordinate this with the convention we use for the path less route support!

brandonroberts commented 1 year ago

I think if it doesn't create too much confusion to have custom names, it should be fine to prefix with an underscore.

I agree about the coordination

cc @markostanimirovic

markostanimirovic commented 1 year ago

I like the idea!

There is another option that we can consider to enable this feature - using custom extensions for pages like in NextJS. This will provide the ability to define a more flexible folder structure, based on user needs. For example, with config { pageExtensions: ['.page.ts'] } and the following folder structure:

routes/
  users.page.ts
  users-meta.resolver.ts
  dashboard.page.ts

Analog router would register two routes: /users and /dashboard.

brandonroberts commented 1 year ago

I like the idea!

There is another option that we can consider to enable this feature - using custom extensions for pages like in NextJS. This will provide the ability to define a more flexible folder structure, based on user needs. For example, with config { pageExtensions: ['.page.ts'] } and the following folder structure:

routes/
  users.page.ts
  users-meta.resolver.ts
  dashboard.page.ts

Analog router would register two routes: /users and /dashboard.

That could be an option also but would require a more significant effort. Analog currently uses Vite's globbing function to get the paths/dynamic imports for the routes.

import.meta.glob(['/src/app/routes/**/*.ts']);

This cannot be dynamic, so we wouldn't be able to configure it through the plugin. We could do something like:

import.meta.glob([
  '/src/app/routes/**/*.ts',
  '!/src/app/routes/**/_route/**/*.ts' // <- ignore these files
]);

I thought about having separate .route.ts files in the beginning, but we already have the routes folder for defining routes, so it felt like extra work to have a src/apps/routes/home.route.ts when there wasn't a notion of different types of files, such as .server.ts for example.

goetzrobin commented 1 year ago

I personally do like the idea of using file endings to make it more obvious what a specific file is doing. But I also think that it is a valid point that it might be redundant having a routes folder and then having to explicitly declare components as routes again.

What about a way to exclude files the same way as folders?

If we stick with parentheses as indicators for pathless routes/folders we can use the underscore as an indicator that something is excluded from the routes config. This could be folders or files.

For files:

routes/
  (home).ts --> '/'
  blog/
      (blog).ts --> '/blog'
      _cta.component.ts --> ignored

For folders:

routes/
  (home).ts --> '/'
  blog/
      (blog).ts --> '/blog'
      [slug].ts --> '/blog/:slug'
      _components/ --> all files inside are ignored
           cta.component.ts
           view-count.component.ts

This would allow us to simply adjust the regex for the globs and also gives a nice 'visual' for the developer that shows which files are considered routes and which are not.

AdditionAddict commented 1 year ago

The downside of underscore for files is it makes for an ugly ordering:

routes/
  dashboard.ts
  users.page.ts
  welcome.ts
  _users-meta.resolver.ts
markostanimirovic commented 1 year ago

This cannot be dynamic, so we wouldn't be able to configure it through the plugin.

Yes, I forgot about this.

goetzrobin commented 1 year ago

@AdditionAddict I do agree that it would definitely be a trade off. My counter argument is that you should then group these files in a folder:

routes/
  dashboard.ts
  welcome.ts
  users/
    (users).ts
    _users-meta.resolver.ts
brandonroberts commented 1 year ago

I'm open to moving to the .page.ts to explicitly define routes. It would keep the glob relatively the same.

The other side is how many users are going to co-locate components next to the route? I'm thinking of Nx as a use case where the components would be off in a library anyway, and the route is just a place to wire up/re-export a page.

Also would this apply to markdown files also? As far as defining a content route as .page.md?

marcus-sa commented 1 year ago

https://remix.run/docs/en/v1/file-conventions/route-files-v2#folders-for-organization

goetzrobin commented 1 year ago

@marcus-sa We were looking at that configuration in https://github.com/analogjs/analog/issues/237#issuecomment-1426409485. I think the remix routing is powerful but can be a little confusing. I do like the simplicity of using (home).ts or [...not-found].ts, etc.

@brandonroberts personally I try to collocate components that I use in only a specific part of the application instead of immediately extracting it into a library. The "follow-the-DOM" approach has been working quite well for me to stay organized.

I think if we adopt the .page file ending it would make sense to rename the routes folder since we would move from implicitly making everything a route to explicitly stating which files are pages and therefore turned into routes.

I think the underscore to exclude a file or folder from becoming a route is a little cleaner.

But then again, the .page path might allow more flexibility in the future...

Definitely an interesting discussion :)

brandonroberts commented 1 year ago

I'd say add support for .page.ts and change src/app/routes to src/app/pages to not break existing apps. Then we can add a migration guide for the new structure and go from there.

The migration should be straightforward from there.

brandonroberts commented 1 year ago

Opened a PR https://github.com/analogjs/analog/pull/281

goetzrobin commented 1 year ago

@brandonroberts this tweet seems relevant and reassuring that the approach implemented is flexible and less error prone than others might have been! https://twitter.com/dan_abramov/status/1630573034120650761?s=20

brandonroberts commented 1 year ago

It's definitely valid! It's all about the trade-offs willing to be made and how they play out over time 😄