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

chore: simplify adder interface #89

Merged
merged 10 commits into from
Oct 11, 2024
Merged

chore: simplify adder interface #89

merged 10 commits into from
Oct 11, 2024

Conversation

manuel3108
Copy link
Member

@manuel3108 manuel3108 commented Oct 11, 2024

Relates #85 , but does not close it.
Goal here is to provide a simpler adder interface, without touching the actual content manipulation.

current decisions, feel free to obey:

  • keep logo property and logo file, i imagine they could really add a benefit to the docs removed, now that they are all called logo.(svg|webp)
  • keep id property, as dependsOn and runsAfter depend on it. we could change that to use the adder name instead, but that would probably lead to more mistakes as name is changed easier / more often than an id. We could elaborate if it's possible to provide a reference to the adder instead, (if we leave out the community adders for now, see Get rid of the --community flag #84)

tbd

  • see if inlining options is possible (Edit: it's not)
  • move files (we don't need all those unnecessary directories, if they only contain one, or max 2 files.

Copy link

pkg-pr-new bot commented Oct 11, 2024

Open in Stackblitz

pnpm add https://pkg.pr.new/sveltejs/cli/@svelte-cli/ast-tooling@89
pnpm add https://pkg.pr.new/sveltejs/cli/sv@89
pnpm add https://pkg.pr.new/sveltejs/cli/@svelte-cli/core@89

commit: 42f7882

@manuel3108
Copy link
Member Author

manuel3108 commented Oct 11, 2024

This looks like a huge diff, but it really isn't. It's probably better to have a look at individual commits here, as most of the diff is just file renames.

This is basically what I have done

  • moved all files into the adders root directory. All adders had only two files in config and one file in test
  • removed the options.ts file, if the adder had no options
  • As index.ts was now just forwarding to adder.ts, deleted all index.ts and renamed all adder.ts file to index.ts. Then changed export const adder = .... to export default ...
  • Renamed all logos to logo.(svg|webp) so that we can get of that logo property that has no meaning for the adder itself.

I also considered removing the logos entirely, but i think they might be something to have in the docs at some point.

Edit: Oh and I did try to inline the options of the adders that actually have options, but that was throwing me type errors that i wasn't able to figure out. We can always improve that later.

@manuel3108 manuel3108 changed the title chore(DRAFT): simplify adder interface chore: simplify adder interface Oct 11, 2024
packages/adders/_config/official.ts Outdated Show resolved Hide resolved
packages/adders/_config/official.ts Outdated Show resolved Hide resolved
@AdrianGonz97 AdrianGonz97 linked an issue Oct 11, 2024 that may be closed by this pull request
@manuel3108
Copy link
Member Author

Can confirm that this fixes #88

@manuel3108 manuel3108 merged commit bf14f4c into main Oct 11, 2024
5 checks passed
@manuel3108 manuel3108 deleted the simplify-adder-interface branch October 11, 2024 15:31
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.

findRoot only works inside workspace projects
2 participants