Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

fix: updated Svelte and SvelteKit Framework Info to latest #846

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: updated Svelte and SvelteKit Framework Info to latest #846

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ae9b48d
edited svelte and sveltekit framework info to update to the latest vi…
Nov 28, 2022
b93006e
fix: #844 svelte and sveltekit info and update readme
brittneypostma Nov 28, 2022
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
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ also returned.
The following frameworks are detected:

- Static site generators: Gatsby, Hugo, Jekyll, Next.js, Nuxt, Hexo, Gridsome, Docusaurus, Eleventy, Middleman,
Phenomic, React-static, Stencil, Vuepress, Assemble, DocPad, Harp, Metalsmith, Roots, Wintersmith
Phenomic, React-static, Stencil, SvelteKit, Vuepress, Assemble, DocPad, Harp, Metalsmith, Roots, Wintersmith
- Front-end frameworks: create-react-app, Vue, Sapper, Angular, Ember, Svelte, Expo, Quasar
- Build tools: Parcel, Brunch, Grunt, Gulp

Expand Down
2 changes: 1 addition & 1 deletion src/frameworks/svelte-kit.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
},
"build": {
"command": "vite build",
"directory": ".svelte-kit"
"directory": "build"
},
"logo": {
"default": "/logos/svelte-kit/default.svg",
Expand Down
8 changes: 4 additions & 4 deletions src/frameworks/svelte.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@
"configFiles": []
},
"dev": {
"command": "npm run dev",
"port": 5000,
"command": "vite dev",
"port": 5173,
Comment on lines -11 to +12
Copy link

@rendall rendall Nov 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I could be wrong here and so this is a "single comment" and not a "review". Please forgive my exceeding ignorance here, especially if none of this is helpful.

I don't know that this will actually fix #844. The assumption that causes #844 seems to be that a [svelte] dependency implies that a framework server is running (perhaps via svelte-kit).

{
  "detect": {
    "npmDependencies": ["svelte"]
  },
 "dev": {
    "command": "vite dev",
    "port": 5173
  }
}

In any case, this PR assumes that vite is the task runner which is probably not right. There might be a problem with assuming that a framework-based dev server is running at all. Maybe a fallback to a static server?

Edit: I struck out a number of my own incorrect assumptions, below. svelte-kit does indeed require vite and there already is a svelte-kit.json file.

This PR doesn't seem to address that assumption, so much as assume that since svelte-kit is being used, so then vite must also be (I have not used svelte-kit, but I suspect that other bundlers / task runners are possible?)

Could it be confusing that this filename is svelte.json? Would it make more sense to name it svelte-kit.json since the assumption here seems to be svelte-kit?

Copy link
Author

Choose a reason for hiding this comment

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

SvelteKit does require vite and uses it for the dev and build commands. Svelte (vanilla) recommends the usage of the Vite template which also uses Vite for the dev and build commands. I think this is the best option going forward to detect the framework correctly and if there is a custom setup, it will need to be manually configured in the netlify.toml file.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, these values are meant to be the defaults for a framework.

Copy link

Choose a reason for hiding this comment

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

Awesome. All is clear, then!

👍🏻

"pollingStrategies": [{ "name": "TCP" }, { "name": "HTTP" }]
},
"build": {
"command": "npm run build",
"directory": "public"
"command": "vite build",
"directory": "dist"
},
"logo": {
"default": "/logos/svelte-kit/default.svg",
Expand Down