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

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

Closed
Rich-Harris opened this issue Apr 20, 2021 · 45 comments · Fixed by #1370
Closed

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

Rich-Harris opened this issue Apr 20, 2021 · 45 comments · Fixed by #1370
Milestone

Comments

@Rich-Harris
Copy link
Member

Ok hear me out.

I think we closed #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
Copy link
Member

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

@Rich-Harris
Copy link
Member Author

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
Copy link
Member

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

@Rich-Harris
Copy link
Member Author

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

@ivoreis
Copy link
Contributor

ivoreis commented Apr 20, 2021

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
Copy link
Contributor

ivoreis commented Apr 20, 2021

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
Copy link
Member Author

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
Copy link
Member

GrygrFlzr commented Apr 20, 2021

+ 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
Copy link
Member

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
Copy link
Contributor

ivoreis commented Apr 20, 2021

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
Copy link
Member

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
Copy link
Member Author

@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
Copy link

UltraCakeBakery commented Apr 20, 2021

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
Copy link
Contributor

ivoreis commented Apr 20, 2021

@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
Copy link
Member Author

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
Copy link
Member

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

@Conduitry
Copy link
Member

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

@Rich-Harris
Copy link
Member Author

windows: pissing in the punchbowl since 1985

@Rich-Harris
Copy link
Member Author

ok, new proposal!

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

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

@arxpoetica
Copy link
Member

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
Copy link
Member

GrygrFlzr commented Apr 20, 2021

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

@arxpoetica
Copy link
Member

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

@Rich-Harris
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member Author

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

image

image

@dummdidumm
Copy link
Member

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
Copy link
Member

@dummdidumm I'm coming around to this opinion.

@ivoreis
Copy link
Contributor

ivoreis commented Apr 20, 2021

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
Copy link
Member Author

_ 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
Copy link
Contributor

ivoreis commented Apr 20, 2021

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
Copy link
Member Author

feasible, but deeply confusing

@ivoreis
Copy link
Contributor

ivoreis commented Apr 20, 2021

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
Copy link
Member

arxpoetica commented Apr 20, 2021

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
Copy link

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

@arxpoetica
Copy link
Member

. files are sometimes entirely hidden in .gitignore

node_modules
.*
# etc.

@UltraCakeBakery
Copy link

UltraCakeBakery commented Apr 20, 2021

. 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
Copy link
Member Author

afraid that's a non-starter

image

@tretapey
Copy link

tretapey commented Apr 20, 2021

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

@Conduitry
Copy link
Member

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

#1149 (comment)

@tretapey
Copy link

missed that

@sveltejs sveltejs locked and limited conversation to collaborators Apr 20, 2021
@benmccann
Copy link
Member

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: microsoft/vscode#118687

@benmccann benmccann added this to the 1.0 milestone Apr 22, 2021
@benmccann
Copy link
Member

benmccann commented Apr 25, 2021

^ 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
Copy link
Member

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
Copy link
Member Author

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

  • __layout.svelte
  • __layout.reset.svelte
  • __error.svelte
  • _private.js

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
Copy link
Member

GrygrFlzr commented May 1, 2021

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

async init_filewatcher() {
this.cheapwatch = new CheapWatch({
dir: this.config.kit.files.routes,
/** @type {({ path }: { path: string }) => boolean} */
filter: ({ path }) => path.split('/').every((part) => !part.startsWith('_'))
});

Also we may want to have a test for this.

Rich-Harris pushed a commit that referenced this issue May 7, 2021
* rename $layout to __layout, etc - closes #1149

* changeset

* fix cheapwatch options

* error on bad files

* rename test files

* tweak docs

* error on encountering $layout etc

* fix
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants