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

Add playground for development #10391

Merged
merged 10 commits into from
Aug 20, 2023
Merged

Conversation

ghostdevv
Copy link
Member

In the Svelte repo we have a playground for locally testing changes, but we don't here - this adds that. WDYT?

My main issue currently is that there are no types for most modules @sveltejs/kit, $app/stores, etc... I believe this is due to the generate-dts script not being run till kit is released - I don't know the best way to fix this. It's also an issue in the other packages so it's not an issue introduced by this pr from what I can tell.

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. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

@changeset-bot
Copy link

changeset-bot bot commented Jul 16, 2023

⚠️ No Changeset found

Latest commit: 182b0e9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@benmccann
Copy link
Member

I've personally never felt the need as I simply use the kit.svelte.dev site that's already checked in for testing, but I have no objection if others find it useful

@ghostdevv
Copy link
Member Author

I've personally never felt the need as I simply use the kit.svelte.dev site that's already checked in for testing, but I have no objection if others find it useful

I've also sometimes used that, but since there is a lot going on there I find it can be harder to test stuff sometimes

@dummdidumm
Copy link
Member

I'm in favor of this change. I use kit.svelte.dev often for this but if also often gets in the way with its prerendering pipeline so I delete stuff temporarily there. This would avoid that.

@benmccann benmccann changed the title Add playground for development chore: add playground for development Jul 18, 2023
@benmccann
Copy link
Member

this PR should probably update https://github.com/sveltejs/kit/blob/master/CONTRIBUTING.md. but otherwise lgtm

@ghostdevv ghostdevv changed the title chore: add playground for development Add playground for development Jul 18, 2023
@ghostdevv
Copy link
Member Author

I added a note in the contributing.md @benmccann

RE: the types problem, is that just me? if not is there something we can do about it, or is it a "this is fine" situation?

@gtm-nayan
Copy link
Contributor

Maybe a good idea to also add a file with these contents:

{
	"pnpm": {
		"overrides": {
			"@sveltejs/kit": "../kit/packages/kit",
			"@sveltejs/adapter-auto": "../kit/packages/adapter-auto",
			"@sveltejs/adapter-cloudflare": "../kit/packages/adapter-cloudflare",
			"@sveltejs/adapter-cloudflare-workers": "../kit/packages/adapter-cloudflare-workers",
			"@sveltejs/adapter-netlify": "../kit/packages/adapter-netlify",
			"@sveltejs/adapter-node": "../kit/packages/adapter-node",
			"@sveltejs/adapter-static": "../kit/packages/adapter-static",
			"@sveltejs/adapter-vercel": "../kit/packages/adapter-vercel"
		}
	}
}

so that there's an easily accessible copypasta when cloning repros. Not sure if there's a better place for it outside the playground folder

@ghostdevv
Copy link
Member Author

Maybe a good idea to also add a file with these contents:

We already have it in the CONTRIBUTING.md so maybe there? Though not sure if we would want to have a list of all packages imo

@gtm-nayan
Copy link
Contributor

gtm-nayan commented Aug 2, 2023

If I understood the description correctly, Geoff's PR to generate the types on postinstall should've fixed the types issue.

@ghostdevv ghostdevv marked this pull request as ready for review August 7, 2023 13:27
@ghostdevv
Copy link
Member Author

Yep #10301 fixed that problem! Though I do wonder if we should watch for changes - at any rate the playground itself is good to go

@dummdidumm
Copy link
Member

Should we put this into sites instead? It's somewhat of a site, after all. What are other's opinions on this?

@ghostdevv
Copy link
Member Author

Should we put this into sites instead? It's somewhat of a site, after all. What are other's opinions on this?

The main reason I put it in packages to align with svelte and other monorepo setups in the ecosystem

@benmccann
Copy link
Member

packages does feel a bit off to me. I've seen a lot of repos just put it in the root directory:

@ghostdevv
Copy link
Member Author

I like the idea of putting it in the root directory

@benmccann
Copy link
Member

I think I like @dummdidumm's suggestion of putting it in sites actually. It might be a bit more organized that way. And then it'll feel less silly that we have a sites directory with only one thing in it 😆

@ghostdevv
Copy link
Member Author

I think I like @dummdidumm's suggestion of putting it in sites actually. It might be a bit more organized that way. And then it'll feel less silly that we have a sites directory with only one thing in it laughing

My only worry is that it'll be tucked away - and while it literally is a site, it's not really a site in the way I imagine that folder was intended. Putting it in packages has always felt weird to me but I have done it as that's what others done, but if vite is putting it at the root that would be my new preferred solution

@benmccann
Copy link
Member

In the Svelte 5 codebase we have a few playgrounds that we just moved under a playgrounds directory. We should probably do the same here for consistency. This could be called playgrounds/basic to match what we've done there where we also have one called basic

tracking in CONTRIBUTING.md
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
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.

4 participants