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

chore(create-app): update svelte templates #2611

Merged
merged 3 commits into from
Mar 24, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions packages/create-app/template-svelte-ts/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,33 @@ This template should help get you started developing with Svelte and TypeScript
## Need an official Svelte framework?

Check out [SvelteKit](https://github.com/sveltejs/kit#readme), which is also powered by Vite. Deploy anywhere with its serverless-first approach and adapt to various platforms, with out of the box support for TypeScript, SCSS, and Less, and easily-added support for mdsvex, GraphQL, PostCSS, Tailwind CSS, and more.

## Technical considerations

**Why use this over SvelteKit?**

- SvelteKit is still a work-in-progress.
- It currently does not support the pure-SPA use case.
- It brings its own routing solution which might not be preferable for some users.
- It is first and foremost a framework that just happens to use Vite under the hood, not a Vite app.
`vite dev` and `vite build` wouldn't work in a SvelteKit environment, for example.

This template contains as little as possible to get started with Vite + TypeScript + Svelte, while taking into account the developer experience with regards to HMR and intellisense. It demonstrates capabilities on par with the other `create-app` templates and is a good starting point for beginners dipping their toes into a Vite + Svelte project.

Should you later need the extended capabilities and extensibility provided by SvelteKit, the template has been structured similarly to SvelteKit so that it is easy to migrate.

**Why `global.d.ts` instead of `compilerOptions.types` inside `jsconfig.json` or `tsconfig.json`?**

Setting `compilerOptions.types` shuts out all other types not explicitly listed in the configuration. Using triple-slash references keeps the default TypeScript setting of accepting type information from the entire workspace, while also adding `svelte` and `vite/client` type information.

**Why include `.vscode/extensions.json`?**

Other templates indirectly recommend extensions via the README, but this file allows VS Code to prompt the user to install the recommended extension upon opening the project.

**Why enable `allowJs` in the TS template?**

While `allowJs: false` would indeed prevent the use of `.js` files in the project, it does not prevent the use of JavaScript syntax in `.svelte` files. In addition, it would force `checkJs: false`, bringing the worst of both worlds: not being able to guarantee the entire codebase is TypeScript, and also having worse typechecking for the existing JavaScript. In addition, there are valid use cases in which a mixed codebase may be relevant.

**Why use an external store, instead of just using local state?**

This allows us to take full advantage of HMR. While `vite-plugin-svelte` does support an option to enable local state saving, it is not recommended, as it is an inherently difficult problem to solve without external stores. Changes to the local state definition can make it unclear what the intended HMR behavior is.
6 changes: 3 additions & 3 deletions packages/create-app/template-svelte-ts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
"serve": "vite preview"
},
"devDependencies": {
"@svitejs/vite-plugin-svelte": "^0.11.1",
"@sveltejs/vite-plugin-svelte": "next",
"svelte": "^3.35.0",
"svelte-preprocess": "^4.6.9",
"typescript": "^4.2.3",
"vite": "^2.1.0"
"vite": "^2.1.2"
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
outline: none;
width: 200px;
font-variant-numeric: tabular-nums;
cursor: pointer;
}

button:focus {
Expand Down
14 changes: 1 addition & 13 deletions packages/create-app/template-svelte-ts/src/lib/hmr-stores.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// Customized HMR-safe stores
// Based off https://github.com/svitejs/svite/blob/ddec6b9/packages/playground/hmr/src/stores/hmr-stores.js
// Use external stores to retain value even after HMR
import { writable } from 'svelte/store'
import type { Writable } from 'svelte/store'

Expand All @@ -8,14 +7,3 @@ let stores: Record<string, Writable<any>> = {}
export function getStore<T>(id: string, initialValue: T): Writable<T> {
return stores[id] || (stores[id] = writable(initialValue))
}
Copy link
Member

Choose a reason for hiding this comment

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

Would it be best if we only export a counter store? This might probably break HMR, but it's weird to have this pattern in the starter template.

Or we can also simplify this and have a normal count variable in Counter.svelte with // @hmr:keep per rixo's idea in Vite Land?

Copy link
Contributor

Choose a reason for hiding this comment

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

this getStore function originated from a playground in svite where i used it for testing purposes. I think it is overkill in a minimal starter template and may not even be a good idea in production use in general.

+1 for a more simple solution

Copy link
Member Author

Choose a reason for hiding this comment

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

From what I can tell our options are:

  1. straight up an external writable() store with no id:
    import { writable } from "svelte/store";
    export default writable(0);
    State is preserved across all updates except to store.js itself, which is intuitive, but also doesn't reflect how people use local state in components.
  2. // @hmr:keep on the single counter variable
  3. // @hmr:keep-all on the component
  4. preserveLocalState in vite.config.js - but this was disabled by default for a reason.

With options 2-4, state is lost upon updating App.svelte, as the Counter component is recreated. We're just picking which flavor of poison to go with.

Thoughts @dominikg @rixo?

Copy link
Member Author

Choose a reason for hiding this comment

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

I discussed this further with dominik over Discord and decided on a fifth option: the default of no local state preservation. The crux of the issue is that its complicated enough to hide behind flags, so like @sveltejs/kit it's likely best to keep disabled, and the behavior can be explained in the README.

Copy link
Contributor

Choose a reason for hiding this comment

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

I strongly believe we should go with option 2. It sidesteps most difficulties, and hints just the right amount of information to maximize people's understanding and learnability.

Enabling preserveLocalState by default goes against the grain of "no friction HMR", I believe, because its behaviour is hard to anticipate and predict intuitively. Because of this, it makes HMR feel more fragile and unreliable than it actually is, hence lessening its value for the user (HMR value is all about being confident that your change are reflected the same way they would be without it, any glitch or unpredictability eats hard into this confidence). Furthermore, the difficulties created by preservation of state are all but exacerbated in Svelte, since any random let variable is a piece of state in Svelte components.

Stores sure are cool and all, but I don't think they have much to do with HMR at this point. State existing in other modules that are not traversed by a HMR bubble is always preserved (otherwise that wouldn't be HMR). Also, there's little doubt that a user of Svelte will soon enough be exposed to lot of documentation and information about them. As such, HMR-wise, I'm pretty neuter to have them or not to have them in a starter example.

On the contrary, most people will never read any bit of HMR docs (we don't want them to have to), and so illustrating that the capacity (preserving local state) exists, and demonstrating the existence of the // @hmr-keep comment is probably the best we can achieve. It non-obstructively hints people about the situation, giving them some hook for something to search about, if they're so inclined.

Other magic HMR comments like // @hmr:keep-all are kinda out of scope for a starter, to me... People will find about them, googling for // @hmr:keep, if they're interested into it. Otherwise, they're not so useful I guess. And HMR is not about HMR, it's about a nice dev experience, and so less docs / information / noise about it is probably the better.

Copy link
Contributor

Choose a reason for hiding this comment

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

@GrygrFlzr To be clear, my last comment was started and sent before your last one, that I just discover. I still personally believe having the behaviour and magic comment trick illustrated rather than documented would probably be the best trade off, but I'm rather good with any direction this can take, except for enabling preserveLocalState by default. This one has really proven confusing and detrimental to the HMR experience in the past, and I much more prefer having the occasional user that is disappointed not to have their counter keep its value, which is really a minor inconvenience, than risking to feed and grow a general sentiment of "yet another broken HMR".


// preserve the store across HMR updates
if (import.meta.hot) {
if (import.meta.hot.data.stores) {
stores = import.meta.hot.data.stores
}
import.meta.hot.accept()
import.meta.hot.dispose(() => {
import.meta.hot.data.stores = stores
})
}
4 changes: 2 additions & 2 deletions packages/create-app/template-svelte-ts/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@
"checkJs": true
},
/**
* Use globals.d.ts instead of compilerOptions.types
* Use global.d.ts instead of compilerOptions.types
* to avoid limiting type declarations.
*/
"include": ["globals.d.ts", "src/**/*.ts", "src/**/*.js", "src/**/*.svelte"]
"include": ["src/**/*.d.ts", "src/**/*.ts", "src/**/*.js", "src/**/*.svelte"]
}
2 changes: 1 addition & 1 deletion packages/create-app/template-svelte-ts/vite.config.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { defineConfig } from 'vite'
import svelte from '@svitejs/vite-plugin-svelte'
import svelte from '@sveltejs/vite-plugin-svelte'

// https://vitejs.dev/config/
export default defineConfig({
Expand Down
30 changes: 30 additions & 0 deletions packages/create-app/template-svelte/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,33 @@ This template should help get you started developing with Svelte in Vite.
## Need an official Svelte framework?

Check out [SvelteKit](https://github.com/sveltejs/kit#readme), which is also powered by Vite. Deploy anywhere with its serverless-first approach and adapt to various platforms, with out of the box support for TypeScript, SCSS, and Less, and easily-added support for mdsvex, GraphQL, PostCSS, Tailwind CSS, and more.

## Technical considerations

**Why use this over SvelteKit?**

- SvelteKit is still a work-in-progress.
- It currently does not support the pure-SPA use case.
- It brings its own routing solution which might not be preferable for some users.
- It is first and foremost a framework that just happens to use Vite under the hood, not a Vite app.
`vite dev` and `vite build` wouldn't work in a SvelteKit environment, for example.

This template contains as little as possible to get started with Vite + Svelte, while taking into account the developer experience with regards to HMR and intellisense. It demonstrates capabilities on par with the other `create-app` templates and is a good starting point for beginners dipping their toes into a Vite + Svelte project.

Should you later need the extended capabilities and extensibility provided by SvelteKit, the template has been structured similarly to SvelteKit so that it is easy to migrate.

**Why `global.d.ts` instead of `compilerOptions.types` inside `jsconfig.json` or `tsconfig.json`?**

Setting `compilerOptions.types` shuts out all other types not explicitly listed in the configuration. Using triple-slash references keeps the default TypeScript setting of accepting type information from the entire workspace, while also adding `svelte` and `vite/client` type information.

**Why include `.vscode/extensions.json`?**

Other templates indirectly recommend extensions via the README, but this file allows VS Code to prompt the user to install the recommended extension upon opening the project.

**Why enable `checkJs` in the JS template?**

It is likely that most cases of changing variable types in runtime are likely to be accidental, rather than deliberate. This provides advanced typechecking out of the box. Should you like to take advantage of the dynamically-typed nature of JavaScript, it is trivial to change the configuration.

**Why use an external store, instead of just using local state?**

This allows us to take full advantage of HMR. While `vite-plugin-svelte` does support an option to enable local state saving, it is not recommended, as it is an inherently difficult problem to solve without external stores. Changes to the local state definition can make it unclear what the intended HMR behavior is.
4 changes: 2 additions & 2 deletions packages/create-app/template-svelte/jsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@
"checkJs": true
},
/**
* Use globals.d.ts instead of compilerOptions.types
* Use global.d.ts instead of compilerOptions.types
* to avoid limiting type declarations.
*/
"include": ["globals.d.ts", "src/**/*.js", "src/**/*.svelte"]
"include": ["src/**/*.d.ts", "src/**/*.js", "src/**/*.svelte"]
}
6 changes: 3 additions & 3 deletions packages/create-app/template-svelte/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
"serve": "vite preview"
},
"devDependencies": {
"@svitejs/vite-plugin-svelte": "^0.11.1",
"@sveltejs/vite-plugin-svelte": "next",
"svelte": "^3.35.0",
"vite": "^2.1.0"
"vite": "^2.1.2"
}
}
}
1 change: 1 addition & 0 deletions packages/create-app/template-svelte/src/lib/Counter.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
outline: none;
width: 200px;
font-variant-numeric: tabular-nums;
cursor: pointer;
}

button:focus {
Expand Down
14 changes: 1 addition & 13 deletions packages/create-app/template-svelte/src/lib/hmr-stores.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// Customized HMR-safe stores
// Based off https://github.com/svitejs/svite/blob/ddec6b9/packages/playground/hmr/src/stores/hmr-stores.js
// Use external stores to retain value even after HMR
import { writable } from 'svelte/store'

/**
Expand All @@ -16,14 +15,3 @@ let stores = {}
export function getStore(id, initialValue) {
return stores[id] || (stores[id] = writable(initialValue))
}

// preserve the store across HMR updates
if (import.meta.hot) {
if (import.meta.hot.data.stores) {
stores = import.meta.hot.data.stores
}
import.meta.hot.accept()
import.meta.hot.dispose(() => {
import.meta.hot.data.stores = stores
})
}
2 changes: 1 addition & 1 deletion packages/create-app/template-svelte/vite.config.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { defineConfig } from 'vite'
import svelte from '@svitejs/vite-plugin-svelte'
import svelte from '@sveltejs/vite-plugin-svelte'

// https://vitejs.dev/config/
export default defineConfig({
Expand Down