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

feat: Sveltekit templates #200

Merged
merged 16 commits into from Oct 4, 2022
Merged

feat: Sveltekit templates #200

merged 16 commits into from Oct 4, 2022

Conversation

melMass
Copy link
Contributor

@melMass melMass commented Oct 1, 2022

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Docs
  • New Binding issue #___
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:
    • Template

Does this PR introduce a breaking change?

  • Yes, and the changes were approved in issue #___
  • No

Checklist

  • When resolving issues, they are referenced in the PR's title (e.g fix: remove a typo, closes #___, #___)
  • A change file is added if any packages will require a version bump due to this PR per the instructions in the readme.
  • I have added a convincing reason for adding this feature, if necessary

Other information

This is a WIP that I will finish either today or tomorrow:

  • Add the missing rust side (enums etc..) as per this
  • Add the Svelte-Kit TS template

Question for Maintainers (cross asked on Discord)

  • Can the templates have a splash screen? In the case of Svelte-Kit the approach is a bit different so it might be interesting.. Or it's maybe more suited for an example than a template maybe.

Answered:

No, templates should be kept as simple as possible and all match oob

@melMass melMass marked this pull request as ready for review October 1, 2022 14:00
- Remove unused route group
- Should fix the CI
- Closer to the barebones SK template.
Copy link
Member

@amrbashir amrbashir left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution. Please move favicon.png file to the _assets_ folder and include it in the _cta_manifest_ file

.changes/svelte-kit-templates.md Outdated Show resolved Hide resolved
packages/cli/fragments/fragment-svelte-kit-ts/src/app.d.ts Outdated Show resolved Hide resolved
packages/cli/fragments/fragment-svelte-kit/src/app.d.ts Outdated Show resolved Hide resolved
packages/cli/src/template.rs Outdated Show resolved Hide resolved
@melMass melMass requested a review from amrbashir October 3, 2022 19:20
packages/cli/fragments/fragment-svelte-kit-ts/src/app.d.ts Outdated Show resolved Hide resolved
packages/cli/src/package_manager.rs Outdated Show resolved Hide resolved
packages/cli/fragments/fragment-svelte-kit/src/app.d.ts Outdated Show resolved Hide resolved
@amrbashir
Copy link
Member

run pnpm format and cargo fmt --all before commiting

@melMass
Copy link
Contributor Author

melMass commented Oct 3, 2022

@amrbashir Done the edits, the only thing I find strange is splitting them in the CLI options no? (sveltekit at the end of the list)
image

@amrbashir
Copy link
Member

Templates are always added to the bottom of the list.

@melMass melMass requested a review from amrbashir October 4, 2022 09:16
Copy link
Member

@amrbashir amrbashir left a comment

Choose a reason for hiding this comment

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

Thank you

@amrbashir amrbashir merged commit 0b09cc1 into tauri-apps:dev Oct 4, 2022
@JDR42
Copy link
Contributor

JDR42 commented Nov 24, 2022

@melMass This is great to get SvelteKit in the mix, thank you! However, the template might need a fix. Using appWindow throws ReferenceError: window is not defined (when running dev server, i.e., tauri dev).

For example:

//  src/routes/+page.svelte

import { appWindow } from '@tauri-apps/api/window';
...
await appWindow.show();
...

Error:

window is not defined
ReferenceError: window is not defined
    at Object.<anonymous> (D:\Projects\playground\tauri3\node_modules\@tauri-apps\api\window.cjs:1:10927)
    at Module._compile (node:internal/modules/cjs/loader:1126:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1180:10)
    at Module.load (node:internal/modules/cjs/loader:1004:32)
    at Function.Module._load (node:internal/modules/cjs/loader:839:12)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/translators:170:29)
    at ModuleJob.run (node:internal/modules/esm/module_job:193:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:533:24)
    at async nodeImport (file:///D:/Projects/playground/tauri3/node_modules/vite/dist/node/chunks/dep-67e7f8ab.js:53137:21)
10:28:27 PM [vite-plugin-svelte] ssr compile done.

This is a known and expected SvelteKit response which happens when SSR is not disabled properly.

So... is SvelteKit's SSR disabled properly in the template? Or perhaps this is a problem with @sveltejs/adapter-static? Is this worth creating an issue for, or must we wait for SvelteKit 1.0?

Again, thank you!

@melMass
Copy link
Contributor Author

melMass commented Nov 24, 2022

AFAIK this is expected behavior?
Even with SSR off the code is "converted" by svelte compiler.

So I would just move this in onMount

I'm maybe wrong as things moves fast in SK, I contributed this when they announced that there should not be major changes to the API for 1.0

@JDR42
Copy link
Contributor

JDR42 commented Nov 24, 2022

@melMass I found out that the new way to disable SSR is to add:

export const ssr = false; in /src/routes/+layout.ts

This fixes the issue. The issue and solution are described here.

It seems to me this should be the default behavior in tauri for the sveltekit template. Do you think I should create a PR for this?

@melMass
Copy link
Contributor Author

melMass commented Nov 24, 2022

@melMass I found out that the new way to disable SSR is to add:

export const ssr = false; in /src/routes/+layout.ts

This was already the case at the time (packages/cli/fragments/fragment-svelte-kit-ts/src/routes/+layout.ts) but maybe they changed the way they are triggered IIRC pre-render induced ssr off. And I use this template in a prod app

Do you think I should create a PR for this?

But yes of course if you think it should be done differently don't hesitate to PR the edit!

JDR42 pushed a commit to JDR42/create-tauri-app that referenced this pull request Nov 24, 2022
Turns off SSR by default, which prevents usage of `appWindow`.

Discussion: tauri-apps#200 (comment)
Issue & solution: https://github.com/sveltejs/kit/tree/master/packages/adapter-static#turn-off-ssr
JDR42 pushed a commit to JDR42/create-tauri-app that referenced this pull request Nov 24, 2022
amrbashir pushed a commit that referenced this pull request Nov 24, 2022
…eltekit templates #200") (#241)

* Update +layout.ts (SvelteKit TS template)

Turns off SSR by default, which prevents usage of `appWindow`.

Discussion: #200 (comment)
Issue & solution: https://github.com/sveltejs/kit/tree/master/packages/adapter-static#turn-off-ssr

* Fix: disable SSR by default in SvelteKit template

Turns off SSR by default, which prevents usage of `appWindow`.

Discussion: #200 (comment)
Issue & solution: https://github.com/sveltejs/kit/tree/master/packages/adapter-static#turn-off-ssr

* changefile
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.

None yet

3 participants