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: supabase #45

Merged
merged 79 commits into from
Oct 9, 2024
Merged

feat: supabase #45

merged 79 commits into from
Oct 9, 2024

Conversation

manuel3108
Copy link
Member

@manuel3108 manuel3108 commented Sep 28, 2024

Notes:

  • shoutout to @mstibbard for the initial pr against svelte-add. I tried patching in his changes, which obviously only worked to some degree. But at least git recognizes that he authored those commits now
  • adds a new scripts section to the adder config. Can be used to execute external npx commands and can be used together with our file-based approach (also initially implement by @mstibbard)
  • removes integrationType: 'external' and integrationType itself from the config, as it now got kinda redundant.
  • the only adder using integrationType: 'external' was storybook. it was migrated to use the new scripts section

mstibbard and others added 30 commits September 27, 2024 21:02
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
Co-authored-by: Manuel <30698007+manuel3108@users.noreply.github.com>
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
Copy link
Member

@AdrianGonz97 AdrianGonz97 left a comment

Choose a reason for hiding this comment

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

this thing is an absolute behemoth. unfortunately i ran out of time today as well, but i'll aim to finish up in the morning!

i'd also like to add that i'm starting to have some reservations about the inclusion of this adder though. the many features that it provides are very cool and i can see how someone can move fast with it, but it's also fairly complex!

i worry that we may be leaning too far into the "opinionated for a particular product" territory (i.e. the reason we don't include any component libraries).

i wonder if this would be better suited as a community adder? im not sure yet

packages/core/files/workspace.ts Outdated Show resolved Hide resolved
packages/cli/commands/add.ts Show resolved Hide resolved
@manuel3108
Copy link
Member Author

I do agree that there is a lot to digest here. The thing is that probably about 60% of the code belong to the demo. All the other options have a pretty low impact code wise iirc.

I do not think we should make it a community adder at this point. We can always try to find someone later, I do think having it is a benefit.

As for the code I was thinking this morning, if it would be smart to have another file called demo.ts or something which also declared a variable with the defineAdderConfig helper function. Then we could spread in the relevant parts as needed, that should already make it easier to reason about the code

@AdrianGonz97
Copy link
Member

AdrianGonz97 commented Oct 7, 2024

As for the code I was thinking this morning, if it would be smart to have another file called demo.ts or something which also declared a variable with the defineAdderConfig helper function. Then we could spread in the relevant parts as needed, that should already make it easier to reason about the code

i like this idea, though we should keep it pretty simple. we shouldnt need any helper functions if we just type annotate it:

// demos.ts
export const demos: FileType<typeof options>[] = [
	{
		name: () => 'some-demo-file.svelte',
		contentType: 'svelte',
		content: () => { ... }
	},
	...
];

@manuel3108
Copy link
Member Author

This was from my phone this morning, where i was assuming we also have some packages that are only necessary for the demo, but that's not the case.
Working on this

@benmccann
Copy link
Member

I wonder if it's possible to shrink down the demo at all. I can't really tell as I'm on my phone, but could we just put an empty file with a comment showing where to put migrations instead of making a notes table and for an example query maybe hit some built-in table? The potential wrinkle on the last part is I'm not sure if the user would have permission to hit the system catalogs or not, but if there is one they have access to it could help simplify things

@manuel3108
Copy link
Member Author

Just had a look at this but i doubt this possible, if we want to keep the demo working, which kinda is the goal

@AdrianGonz97
Copy link
Member

AdrianGonz97 commented Oct 9, 2024

since we've decided that the supabase adder would be better as a community adder, we'll need to merge everything in this PR besides the supabase specific code.

we could either break it out into a separate PR or we can delete the adder here and merge this one. i'm fine with either, so whichever is easiest!

@manuel3108 manuel3108 merged commit 8f3c23b into main Oct 9, 2024
5 checks passed
@manuel3108 manuel3108 deleted the feat/supabase branch October 9, 2024 15:23
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