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

Sverdle #6979

Merged
merged 50 commits into from
Sep 28, 2022
Merged

Sverdle #6979

merged 50 commits into from
Sep 28, 2022

Conversation

Rich-Harris
Copy link
Member

@Rich-Harris Rich-Harris commented Sep 22, 2022

Playable here: https://kit-git-sverdle-svelte.vercel.app/sverdle

The /todos app in the default template is a nice idea, but it's a bit misleading. People understandably imagine that the back end runs locally and then are surprised by the latency.

This PR replaces it with a Wordle clone that keeps all the game logic and data on the server, and persists user state using cookies.

It's not 100% finished — there's a subtle bug with the keyboard showing some letters as wrong instead of right-letter-wrong-place, and it needs some instructions and probably some style tweaks. I'd also love to get some help on the a11y front (cc @geoffrich!)

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

@changeset-bot
Copy link

changeset-bot bot commented Sep 22, 2022

🦋 Changeset detected

Latest commit: ecc109d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
create-svelte Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Rich-Harris
Copy link
Member Author

(Also: we might think this thing is too complex to have in a demo app, in which case I'm certainly open to alternatives. The code can probably get simplified a bit though, it was written under conference deadline conditions.)

@Conduitry
Copy link
Member

One of my initial suggestions would be whether we can stick the word lists in some external package. That's a lot of stuff to ship with everyone using create-svelte whether or not they opt to use the Sverdle example.

@Rich-Harris
Copy link
Member Author

counterpoint: it's easier to delete the entire sverdle directory than to do that and hunt around in package.json for unused packages. case in point: i forgot to remove the cookie and @lukeed/uuid packages even though they are surplus to requirements

@gtm-nayan
Copy link
Contributor

gtm-nayan commented Sep 23, 2022

counterpoint: it's easier to delete the entire sverdle directory than to do that and hunt around in package.json for unused packages. case in point: i forgot to remove the cookie and @lukeed/uuid packages even though they are surplus to requirements

Is the demo app supposed to be something people modify and build their own app upon? I had thought it was just something to learn from and then they would use the skeleton template when creating a new app.

How about downloading the words list from kit.svelte.dev/somewhere or raw.githubusercontent.... and then cache it on disk when it first runs? Could be done using an inline vite plugin or in create-svelte only if the demo template is selected, that way consumers of other templates wouldn't be affected.

@martinbeentjes
Copy link

martinbeentjes commented Sep 23, 2022

TL;DR - Keep Sverdle as a demo application, introduce some middleway of a create-svelte-kit-app which creates all directories useful, add maybe readme's and maybe an advanced hello world.

The templates are now a skeleton, a library skeleton and the todo app. Maybe to have Sverdle as a 'template' is misleading as it is complex. However, this is an useful application to look at. It answers my questions I had when I started with SvelteKit.

I want to propose an enhanced create-svelte-app. Maybe with a bit more flexibility on the components to import. Implicitly this is just a simplified general template. For example, with the directories, and possible a README there? (or does that make it bloated?) So you have the structure of the full fledged SvelteKit app, but the cleanness of the skeleton.

@dummdidumm
Copy link
Member

Having more than two versions would be too much to choose as a newcomer ("what do I pick?") and maintain in my opinion. To be clear the Sverdle would replace the TODO example.

For me the question comes down to finding a good middle ground for the app that

  • showcases most of the basic features
  • is easy to dig into and understand
  • is shiny, i.e. you like to browse it a few minutes, it's not just a "here's what you can do, look at the code, the app is boring"

@bato3
Copy link

bato3 commented Sep 23, 2022

The TODO application is so good that everyone understands its idea.

Sverdle - WTF? First I need to find out what it is. And then only check how it works in SK.

I agree with one thing: TODO in its current form should disappear.

BTW:

I miss an example of logging in, storing data in the session, connecting to the database, (group), [slug], REST-API ...

I suggest that there are 2 applications to choose from:

@Rich-Harris
Copy link
Member Author

Is the demo app supposed to be something people modify and build their own app upon? I had thought it was just something to learn from and then they would use the skeleton template when creating a new app.

The latter, though a lot of people do use the 'default' template (which should probably be renamed to 'demo') when creating repros etc. (That's probably our fault, since we publish https://github.com/sveltejs/kit-template-default but not https://github.com/sveltejs/kit-template-skeleton.)

Sverdle - WTF? First I need to find out what it is. And then only check how it works in SK.

When it's finished it'll have instructions and a more self-explanatory UI, which will hopefully address that

I miss an example of logging in, storing data in the session, connecting to the database, (group), [slug], REST-API ...

I hear you, for sure. The trouble is that any not-completely-unrealistic demonstration of those features requires a back end, and then we're back at the same problem we had with /todo — we can either provide one for you a la api.svelte.dev, or we add a bunch of hoops you have to jump through if you want to actually deploy the thing. I definitely think there's a place for those sorts of examples (somewhere on learn.svelte.dev ideally, eventually) but it's probably not the demo app

@Rich-Harris
Copy link
Member Author

the dictionary file isn't suitable for example

It's fine! It's completely trivial compared to (for example) the contents of node_modules

tic-tac-toe or 4 in a line

the trouble is those games are really boring :)

it demonstrates the CRUD ability

I'm not sure what this means to be honest — SvelteKit doesn't have 'CRUD ability', it just has arbitrary actions. If we demonstrate CRUD we need a back end, which brings back the issue this PR was designed to address. A switchable back end adds complexity and makes the example sufficiently unrealistic that it starts to lose value.

always nice to have an indication for the user, that it's loading the result from server

yeah, we could add something. not sure what the best UI would be

@Tal500
Copy link
Contributor

Tal500 commented Sep 28, 2022

the dictionary file isn't suitable for example

It's fine! It's completely trivial compared to (for example) the contents of node_modules

OK

tic-tac-toe or 4 in a line

the trouble is those games are really boring :)

🧐🤨🙄 OK, matter of test. I just thought it's easier and more classic for a startpoint, in the logic of that the normal user doesn't use svelte-create to generate a game he likes, he goes to official sveltekit demos for it😅. Not sure if we need a game at all for a startpoint templats though, although I really would like to see these stuffs on demos.

it demonstrates the CRUD ability

I'm not sure what this means to be honest — SvelteKit doesn't have 'CRUD ability', it just has arbitrary actions. If we demonstrate CRUD we need a back end, which brings back the issue this PR was designed to address. A switchable back end adds complexity and makes the example sufficiently unrealistic that it starts to lose value.

I'm not good at word choice here, I meant by "CRUD abilities" the meaning of "CRUD startpoint template", which is far more useful to the user. For example, I used this "TODO" generated route, for checking how Kit team chose to break the API everytime🤣. I believe that it will be useful, at least for me, to see "only the code I need to see" in the TODO route for remembering again the suggested CRUD structure in Kit.

@dummdidumm
Copy link
Member

Could we reduce the possible (allowed) words so the files aren't that huge? Also, what are the allowed words for? Why can't we just compare against the chosen word?

Not without making the game less fun to play! The allowed words are less common English words that are valid submissions, but won't ever be chosen as the answer.

I still don't get it. Why check against a list of allowed words? Why not do "ok you didn't hit any of the characters in this word -> wiggle"? Is it because you could have a valid word without any shared characters? How often does this happen? Is it worth keeping this for the additional thousands of words we need for that?

The new version is much easier to follow, if I do say so myself. The page is basically one screen's worth of script, another screen's worth of markup, then a bunch of styles. It's not quite as simple as /todos but it's a lot more fun/unique and the use of cookies as a datastore feels much more appropriate in this context, so I reckon we can get away with it!

Yeah it reads much better now (although the question stays if we are just too familiar already if it by now). Btw the "how to play" page needs some love 😄 Maybe a section on how it works under the hood would also be good, so that you can already grasp from a high level how it works without looking a the code. Something like "Under the hood this uses regular forms and cookies to implement the game. This means it even works without JavaScript!"

@geoffrich
Copy link
Member

I still need to do an a11y pass on this, but I can make a follow-up issue for that.

// Prettier strips 'unnecessary' parens from .ts files, we need to hack them back in
code = code.replace(/(\/\*\* @type.+? \*\/) (.+?) \/\*\*\*\//g, '$1($2)');

return prettier.format(code, {
Copy link
Member

@dummdidumm dummdidumm Sep 28, 2022

Choose a reason for hiding this comment

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

I remember having the same problem in prettier-plugin-svelte (the parentheses thing) when using the typescript parser (sveltejs/prettier-plugin-svelte#218), using the babel-ts parser fixed it. I wonder though why it doesn't work in this case, because the formatter uses the babel parser, which I would have expected to be a JS parser which should keep the parentheses. Maybe babel-ts is worth a try?

@Rich-Harris
Copy link
Member Author

I still don't get it. Why check against a list of allowed words? Why not do "ok you didn't hit any of the characters in this word -> wiggle"?

The wiggle happens if you enter an invalid word, not if you enter a wrong word

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

Successfully merging this pull request may close these issues.

Remove network calls from demo app SvelteKit demo app 'todos' failing on delete
9 participants