sveltejs / kit

web development, streamlined
https://kit.svelte.dev
MIT License
17.88k stars 1.8k forks source link

Change $layout.svelte to @layout.svelte and [param].svelte to {param}.svelte? #1149

Closed Rich-Harris closed 3 years ago

Rich-Harris commented 3 years ago

Ok hear me out.

I think we closed https://github.com/sveltejs/kit/issues/894 prematurely; it does make sense to use a character that doesn't require special handling, if possible. It's awkward to type src/routes/$layout.svelte on the command line — you get to the $ and either have to reach for the backslash, or curse your lack of forethought in not typing 'src (single quote only, no doubles!) instead of src.

Even if you don't find yourself typing/tab-completing route filenames on the command line frequently, you might find yourself attempting to cmd-click on the filename in your editor's integrated terminal (e.g. in the output from git diff or tsc or whatever) in order to edit the file. At least in VSCode, that doesn't work for filenames containing $layout, probably because something somewhere is replacing it with the layout environment variable.

These problems go away with @.

Related to the cmd-click problem: VSCode doesn't recognise this as a file — if you hover it, it will only highlight the file up to the [ character:

image

image

This, on the other hand...

image

...works perfectly:

image

I should note that this behaviour appears to be specific to VSCode; iTerm reverses the treatment of [bar] and {bar}. But I think it's probably more important that cmd-click works in integrated editor terminals, and VSCode is the most popular one of the bunch.

Aesthetically, @layout.svelte and @error.svelte seem fine, and I actually prefer {param} to [param], not least because it's more aligned with the syntax of Svelte itself.

dummdidumm commented 3 years ago

If $ is changed to @, would that mean aligning the other aliases with it? @lib/.. instead of $lib/.., @app/navigation etc..

Rich-Harris commented 3 years ago

It doesn't need to, since you don't have a reason to type $app/paths etc on the command line (and it wouldn't mean anything in that context anyway). But we could, I guess

dummdidumm commented 3 years ago

Yeah there's no techical reason, more an alignment question

Rich-Harris commented 3 years ago

@GrygrFlzr points out that @ is a nuisance character in a lot of chat contexts (discord, github, twitter). Maybe +layout.svelte?

ivoreis commented 3 years ago

As a vscode user myself I think this change would be great and I'd avoid a considerate amount of URL copy/pasting around. I remember NextJS RFC about this but I don't think the {} option was proposed or discarded at any point.

ivoreis commented 3 years ago

Can someone remind me what is the downside of having layout.svelte without any prefix? Or why do we need to prefix the file in the first place?

Rich-Harris commented 3 years ago

Ha yes, Next and Nuxt explicitly based their dynamic parameter syntax on Sapper — I guess past me accidentally sabotaged them.

The prefix is there because xyz.svelte creates the /xyz route, but layout.svelte doesn't create the /layout route. You need some way to indicate that it's a 'special' file (simply saying 'layout and error are special names' doesn't work, because we might need to add to that list of names over time — as we did recently — and it would be a breaking change if we didn't have a prefix because someone might already be using the name we want to add). In Sapper that was done with the _ prefix, but that's also used for private modules/components, and suffers from the same problem of not being future-proof.

GrygrFlzr commented 3 years ago

+ is somewhat less ergonomic on a QWERTY keyboard and requires finagling your right hand or the combination of left hand shift plus right hand =, versus the current $ being reachable by the left hand. This isn't a common use case if we only apply it to the files, but if we align $app to +app, that's a much more frequently-typed thing that would quite literally be more painful to type.

Also I think it +app/env looks funny, but that's hardly a strong argument.

ignatiusmb commented 3 years ago

I would lean to either # or ~ in that order as a replacement for $.

As for [param] and {param}, both seems fine either way, but the latter still behaves the same on windows cmd (in VSCode).

image

ivoreis commented 3 years ago

Thanks for the clarification @Rich-Harris and I see your point. Looks like we have 2 "special" files (layout, error) that need to be excluded from route creation. It looks like we're saying that the overhead of adding new files to an exclusion list is greater than having a special character to signal this is a 'special' file but at the same time, any special files need to have some code changes associated that would handle those use-cases.

When you refer to a breaking change, is this in regards to SvelteKit, sapper or something else? If we're talking about SvelteKit this current stage might be the right time to have breaking changes (if there even a right time for that 😀)

Is it an option to have a new folder, something like kit (same level as lib/routes) where all these special files would live, this way we'd avoid collisions and we would be able to keep the file name simple?

Thinking about this having a new folder would work for the root files but it wouldn't for nested layouts unless we'd use a similar filesystem structure inside the kit folder, which might not be ideal.

GrygrFlzr commented 3 years ago

Some findings about the VS Code Ctrl+Click behavior on Windows:

src/routes/[dynamic]/[nested]/test.svelte goes to the folder src/routes/[dynamic]/[nested]/test.svelte shows files inside the dynamic folder src/routes/[dynamic]/[nested]/test.svelte goes to the file src/routes/[dynamic]/[nested]/test.svelte does a search for test.svelte files if there's more than one

This works the same for {}.


I would lean to either # or ~ in that order as a replacement for $.

# has the same issue as @ in terms of being annoying on chat/social media platforms (channel names, hashtags), also I think Vite uses it internally so it might break? Not sure about potential issues with ~.

Rich-Harris commented 3 years ago

@ivoreis It's a breaking change for future versions of SvelteKit that add new special files.

It's also just bad design. layout.svelte is a completely different sort of thing than layin.svelte, and that difference should be obvious and explicit, not dependent on knowing specific details about how files are turned into routes. Having a new folder is a non-starter, the whole point is to colocate layouts and errors with the routes they pertain to

UltraCakeBakery commented 3 years ago

Almost all the other solutions mentioned above give me the gut feeling that things won't work elsewhere. I think using _ for layout/error and - for private routes makes the most sense as they are the safest characters to be used. _ and - are almost always allowed on files and in the average command line because _ often gets used as a alphabetic sorting hack and - as a alternative to a space .

ivoreis commented 3 years ago

@Rich-Harris yeah I agree, while I was typing I got to the same conclusion.

So ATM it looks like we have a few options (@, _, $, +) for file special file prefixing, what are the criteria that we'd use to decide the best option?

Rich-Harris commented 3 years ago

For dynamic parameters, it seems that the only plausible characters that don't behave differently between VSCode and iTerm (iTerm makes filenames with [] and () interactive but VSCode doesn't; VSCode makes filenames with {} and <> interactive but iTerm doesn't) are pipes:

src/routes/|dynamic|/|nested|/test.svelte

That becomes interactive (i.e. cmd-clickable) in iTerm and VSCode; I can't guarantee the same is true everywhere, but it seems like the surest bet.

I still think @ is the best option for layout/error files. In github/discord it's very easy to wrap in backticks, and I'm not too concerned about the twitter case. There is good precedent for its use in scoped packages (though incidentally, this is a reason not to use it for @app/* etc, but rather to keep those as $).

GrygrFlzr commented 3 years ago

| designates pipelining in Unix, DOS, and Windows, and are not allowed characters in Windows filenames.

Conduitry commented 3 years ago

\ / : * ? " < > | are all disallowed characters in filenames on Windows.

Rich-Harris commented 3 years ago

windows: pissing in the punchbowl since 1985

Rich-Harris commented 3 years ago

ok, new proposal!

src/routes/%dynamic%/%nested%/index.svelte

Works without backticks, interactive in both iTerm and VSCode, allowed in windows

arxpoetica commented 3 years ago

One more.

src/routes/~dynamic~/~nested~/index.svelte
src/routes/~dynamic~/~nested~/~layout.svelte

Tilde has a nice calming wavy effect. Ergonomically easy to reach.

GrygrFlzr commented 3 years ago

@arxpoetica With the latter variant, layout, error, and any future files we want to implement becomes a special reserved keyword.

arxpoetica commented 3 years ago

@GrygrFlzr yeah, I immediately realized, and deleted my comment—which is why you're yelling into the void.

Rich-Harris commented 3 years ago

Here's an argument for % that I think is solid: you can't use it in a URL pathname (unless it's followed by characters that are being %-encoded). The other characters we've been discussing don't have that property (source). ([] is also safe on that basis, incidentally.) If we used ~ or =, for example, it would make it harder to use them as literals.

We can also reduce the ugliness by 50%, by having %param.svelte instead of %param%.svelte. (We'd need to use both in more complex cases like %param%-foo.svelte or %param%-%bar%.svelte, but those are very much the minority.)

Rich-Harris commented 3 years ago

One major downside to % — Vite barfs on it:

image

If you change the import to be URL-encoded, you get this:

image

Haven't delved into it too much yet but I think this is a bug in Vite — I suspect it should be encoding import URLs when it transforms modules

Rich-Harris commented 3 years ago

Yeah, I think this is something that Vite could address. It's totally fine to have modules with % in the filename:

image

image

dummdidumm commented 3 years ago

I'm indifferent about changing prefixes for stuff like layout files, but I for sure don't like trading [] or {} for another pair of characters. The 80% use case is reading the file names or creating them, not clicking on the file names in some terminal that doesn't know how to read it. So it feels totally wrong for me to optimize for the 20% use case and trade the readability and convention (Sapper, Next etc use []) for something that happens to work on VS Code.

arxpoetica commented 3 years ago

@dummdidumm I'm coming around to this opinion.

ivoreis commented 3 years ago

Have we discarded the underscore (_) option?

src/routes/_dynamic_/_nested_/index.svelte // dynamic path, non-dynamic file
src/routes/_dynamic_/_nested_/file_name.svelte // dynamic path, non-dynamic file
src/routes/_dynamic_/_nested_/file-name.svelte // dynamic path, non-dynamic file
src/routes/_dynamic_/_nested_/_name_.svelte  // dynamic path, dynamic file
src/routes/_dynamic_/_nested_/_var1___var2_.svelte // dynamic path, dynamic file
src/routes/_dynamic_/_nested_/_var1_-_var2_.svelte  // dynamic path, dynamic file
src/routes/_dynamic_/_nested_/_private.svelte  // dynamic path, non-dynamic and private file
src/routes/_dynamic_/_nested_/@layout.svelte // dynamic path, reserved file

_ is a safe character so no encoding required and it seems to work fine vscode/term/browser(devtools)

Screenshot 2021-04-20 at 17 23 33 Screenshot 2021-04-20 at 17 24 50 Screenshot 2021-04-20 at 17 27 09

I do like how expressive [ ] and { } are but at the same time not being able to click-through files and I need to manually find the file causes some frustration. The fact that I need to manually find/goto file it slows down the flow and doesn't feel like a great developer experience (not saying is bad, but not great).

I'm not saying we should change this convention, but if it slightly improves the development experience for everyone, wouldn't we want that @dummdidumm ?

Rich-Harris commented 3 years ago

_ already has meaning (private modules/components), so either we have to find a new solution for that, or we can't use it for this

ivoreis commented 3 years ago

I've added an example that covers that: src/routes/_dynamic_/_nested_/_private.svelte // dynamic path, non-dynamic and private file

dynamic routing is wrapped by _ while private is prefixed. Do you reckon this wouldn't be feasible?

Rich-Harris commented 3 years ago

feasible, but deeply confusing

ivoreis commented 3 years ago

Yeah, it looks like there isn't any obvious solution.

The other option that doesn't need encoding and plays well across multiple systems is the tilde ~ like @arxpoetica suggested earlier (either wrapping src/routes/~dynamic~/~nested~/_private.svelte or prefixing src/routes/~dynamic/~nested/_private.svelte) but none of them is as graceful as a vertical character like [ or { that pleases both windows and unix gods 😅

arxpoetica commented 3 years ago

Honestly, I'm leaning more toward this being a "bug" with IDEs / terminals that provide clickable links. I no longer favor ~ tilde, happy little wave that it is.

UltraCakeBakery commented 3 years ago

How about using . for private modules/components and _ for layouts / errors?

arxpoetica commented 3 years ago

. files are sometimes entirely hidden in .gitignore

node_modules
.*
# etc.
UltraCakeBakery commented 3 years ago

. files are sometimes entirely hidden in .gitignore

node_modules
.*
# etc.

Any freshly created nuxt project always comes with an preconfigured .gitignore based on your projects configuration. SvelteKit could offer a preconfigured .gitignore when creating a fresh svelte project too. If we add all the major . prefixed files that people generally try to target with .* we may be on to something...

Rich-Harris commented 3 years ago

afraid that's a non-starter

image

tretapey commented 3 years ago

What about using for those files? like src/routes/layout.svelte, will it work with vscode and iTerm?

Conduitry commented 3 years ago

What about using for those files? like src/routes/layout.svelte, will it work with vscode and iTerm?

https://github.com/sveltejs/kit/issues/1149#issuecomment-823319922

tretapey commented 3 years ago

missed that

benmccann commented 3 years ago

Here's an issue for the VS Code bug where I posted some details showing the line number where the bug is in case anyone wants to try fixing VS Code instead of changing SvelteKit: https://github.com/microsoft/vscode/issues/118687

benmccann commented 3 years ago

^ would be another option. = as well

I still don't think that _ should be out of the running. I'm not sure why it's an issues that it's used for private modules. We don't want _layout.svelte or _error.svelte to be routable so it seems in keeping with the private aspect of it

Conduitry commented 3 years ago

It's an issue because if we later want to add _foo.svelte with some other special meaning, it's a breaking change because anyone could have already been using it with no special meaning as a component they can import from other pages, and upgrading would then likely break their apps.

Rich-Harris commented 3 years ago

Core team has been bikeshedding this, and we think the best option is to use two underscores for special files:

We don't like any of the alternatives to [params], so we're just going to cross our fingers and hope for a VSCode fix

GrygrFlzr commented 3 years ago

We need to make sure to change this for the manifest files:

https://github.com/sveltejs/kit/blob/3b21b7e558e33885f40b7e09befb4c6c3d86b958/packages/kit/src/core/dev/index.js#L58-L63

Also we may want to have a test for this.