Skip to content
This repository was archived by the owner on Sep 12, 2019. It is now read-only.

Add svelte and sapper to list of detectors #201

Merged
merged 2 commits into from
Jun 14, 2019
Merged

Add svelte and sapper to list of detectors #201

merged 2 commits into from
Jun 14, 2019

Conversation

shortdiv
Copy link
Contributor

- Summary
Now that Svelte 3 is out, we should support svelte projects on Netlify Dev

- Test plan
Run an example svelte project.

Sapper:

npx degit "sveltejs/sapper-template#rollup" my-app
cd my-app
netlify dev

Svelte:

npx degit sveltejs/template my-svelte-project
cd my-svelte-project
netlify dev

- Description for the changelog
Adds Svelte and Sapper to list of detectors

- A picture of a cute animal (not mandatory but encouraged)
elfoDance

@swyxio swyxio merged commit bb6c3d1 into master Jun 14, 2019
env: { ...process.env },
possibleArgsArrs,
urlRegexp: new RegExp(`(http://)([^:]+:)${5000}(/)?`, "g"),
dist: "dist"

Choose a reason for hiding this comment

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

I know this PR is merged already, but I was just wondering about the choice of dist here. The default Svelte template uses a public folder for its static output.

Copy link
Contributor

Choose a reason for hiding this comment

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

i'm not that familiar with svelte, i didnt really check it. i have never liked the choice of dist as the keyword, as we dont care about output, we just care about the location of the _redirects file if it exists. this is a problem we'll have to fix at source

Choose a reason for hiding this comment

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

Thanks for looking at this. The sapper location would be static which you updated correctly, but the Svelte default is public.

// REQUIRED DEPS
if (!hasRequiredDeps(["svelte"])) return false;

/** everything below now assumes that we are within vue */

Choose a reason for hiding this comment

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

Pedantic copy/paste leftover in comment. :)

Copy link
Contributor

Choose a reason for hiding this comment

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

heh

}

return {
type: "vue-cli",

Choose a reason for hiding this comment

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

This should be sapper.

Copy link
Contributor

Choose a reason for hiding this comment

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

indeed

env: { ...process.env },
possibleArgsArrs,
urlRegexp: new RegExp(`(http://)([^:]+:)${3000}(/)?`, "g"),
dist: "dist"

Choose a reason for hiding this comment

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

This should be static.

Copy link
Contributor

Choose a reason for hiding this comment

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

agreed

@swyxio swyxio deleted the add-svelte branch June 17, 2019 05:20
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 this pull request may close these issues.

3 participants