8000 Add a "Paraglide-SvelteKit" Adder by LorisSigrist · Pull Request #520 · svelte-add/svelte-add · GitHub
[go: up one dir, main page]

Skip to content
This repository was archived by the owner on Oct 20, 2024. It is now read-only.

Add a "Paraglide-SvelteKit" Adder #520

Closed
wants to merge 19 commits into from
Closed

Conversation

LorisSigrist
Copy link
@LorisSigrist LorisSigrist commented Jul 31, 2024

Closes #227

This PR adds an Adder for Paraglide-SvelteKit

I had to expand the options on the text-prompts a little to allow for input validation & custom placeholders.

Open Questions

  • Is it possible to dynamically add files based on the options provided by the user?
  • Is it possible to scan the working directory before displaying the prompts (without using hacks like top-level await)

Copy link
changeset-bot bot commented Jul 31, 2024

🦋 Changeset detected

Latest commit: e7d2a59

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

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

// sourceLanguageTag: 'en',
// languageTags: ['en'],
modules: [
'https://cdn.jsdelivr.net/npm/@inlang/message-lint-rule-empty-pattern@latest/dist/index.js',
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure I love the idea of dynamically fetching and running code. Could these be installed in package.json? Though it would be a lot of packages to install... It would be nice if they all lived in a single package

Copy link
Member

Choose a reason for hiding this comment

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

Totally agree!

Choose a reason for hiding this comment

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

@benmccann @manuel3108 would storing plugins directly in an inlang folder be better?

Here is a 3 min loom explainer https://www.loom.com/share/ebb2f7e988744eb4959b1e0965e69f7d

.
└── project.inlang/
    ├── plugins/
    │   ├── i18next.js
    │   ├── m-function-matcher.js
    │   └── inlang-message-format.js
    └── settings.json

Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps what might be nice to do here would be to change https://cdn.jsdelivr.net/npm/@inlang/message-lint-rule-empty-pattern@latest/dist/index.js to https://cdn.jsdelivr.net/npm/@inlang/message-lint-rule-empty-pattern@1.4.8/dist/index.js. Using latest means that you could get a different version each time you run it, which introduces some risk that your project could break due to a change in a plugin and it'd be quite hard to diagnose. If you manually upgraded the plugin and things broke you'd know what caused the error. This is why we use a lockfile with node modules. Losing that confidence is what caught my eye here.

Storing plugins directly in an inlang folder could also be okay. I like the idea of git controlling what version you use. It seems a little harder to update the version that you're using though. E.g. instead of just bumping a version number here, we'd have to download and install a tool to help us get a new version or would have to manually construct the URL to get it from npm and then copy and paste it in. The thing I do like about storing it in git is that it greatly reduces being exposed to the risk of jsdelivr being compromised and also reduces the risk of being impacted by a temporary outage. But I'm not sure I'd go that route as the developer experience doesn't seem as nice.

I think if we could find a way to support npm that would be nicest. Of course that wouldn't work on many of the platforms you support like android/ios, but perhaps it could be a node plugin to load the files from there or the CDN could be used as a fallback on those other platforms where node_modules would be available.

Choose a reason for hiding this comment

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

Losing that confidence is what caught my eye here.

Then let's pin the version. It seems like a good compromise between "building node dependencies would not work in the web and other platforms" and the desire to have reproducible builds.

@LorisSigrist agree?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, pinning the version is a good compromise until we have a solution that works without a CDN

Copy link
pkg-pr-new bot commented Jul 31, 2024

Open in Stackblitz

pnpm add https://pkg.pr.new/svelte-add/svelte-add@520
pnpm add https://pkg.pr.new/svelte-add/svelte-add/@svelte-add/config@520
pnpm add https://pkg.pr.new/svelte-add/svelte-add/@svelte-add/testing-library@520
pnpm add https://pkg.pr.new/svelte-add/svelte-add/@svelte-add/core@520

commit: e7d2a59

Copy link
Member
@manuel3108 manuel3108 left a comment

Choose a reason for hiding this comment

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

Is it possible to dynamically add files based on the options provided by the user?

Not exactly sure what you mean? Do you want to add a file if user selected de-CH as a language? Then you could do something like this:

condition: ({ options }) => options.availableLanguageTags.includes("de-CH"),

Is it possible to scan the working directory before displaying the prompts (without using hacks like top-level await)

Not sure what you are trying to achieve here. We have pre-conditions that you could add to the checks.ts file. They were mainly intended to be used to check if some global tools have been installed (like rust for tauri), but you could probably also use it here. You can find an example implementation in #368


Would you mind sharing a docs link with details what to do after applying the adder? Not sure what do after I applied it :D

// sourceLanguageTag: 'en',
// languageTags: ['en'],
modules: [
'https://cdn.jsdelivr.net/npm/@inlang/message-lint-rule-empty-pattern@latest/dist/index.js',
Copy link
Member

Choose a reason for hiding this comment

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

Totally agree!

8000

export const options = defineAdderOptions({
availableLanguageTags: {
question: 'Which language tags would you like to support?',
Copy link

Choose a reason for hiding this comment

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

I believe this question would better be asked by an inlang cli.

If we allowed each tool to go into details about its own configuration, svelte-add would grow a lot and become overwhelming to the user.

We could think about chaining a call to the cli of the added tool to continue setup.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We have another concept of post-install setup instructions. Maybe this would be a good use of that. Rather than asking a question about what language to setup, when the adder is done running we can instruct the user where to further configure their desired languages.

I don't really love the idea of calling out to another CLI because the user has to answer just as many questions in that case.

Copy link
Author

Choose a reason for hiding this comment

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

I don't think that calling an external CLI will result in a nice(r) user experience

If we allowed each tool to go into details about its own configuration, svelte-add would grow a lot and become overwhelming to the user.

Fine grained tool-specific config should absolutely stay out of svelte-add, but allowing tools to ask high-level questions is likely unavoidable. Many adders already feel the need to ask the user for config that could theoretically be left to the "post-installation instructions".

  • The Tailwind adder asks about adding it's optional typography plugin
  • Drizzle asks about which database you want & if it should run inside a docker container
  • Bootstrap asks about if you want to include it's optional front-end JS bundle

IMO you should have a working app after running "svelte-add [thing]".

If an i18n library doesn't know which languages to add it either:

  • Can't give you a working app
  • Has to add some default language-config which likely doesn't match what the user wants

It then has to instruct the user to find and edit the config. Is this really a better user-experience than just asking?

Copy link

Choose a reason for hiding this comment

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

These questions change over time, adding churn to the maintenance of svelte-add.

Setting up a minimal default configuration and then delegating to a config tool of the added item ensures that this stays decoupled and you are in control of your own ui/dx.

The tool (or tools) can be called in succession at the end of the svelte-add call ala

Successfully added inlang with default config to your app, do you want to customize it now? (Y/n)

Copy link
Author
@LorisSigrist LorisSigrist Aug 5, 2024

Choose a reason for hiding this comment

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

That makes sense, I'm open to a flow like that

I do have an implementation question: where would the post-install customization CLI come from?

If I understand you correctly it would be installed by the adder. How would the versioning of this CLI work? Does the adder install an exact version or just "latest".

  • If exact version: svelte-add ends up with churn anyways since updating the CLI requires updating the adder with a new version
  • If latest: The same svelte-add version may result in different prompts

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's hard to find the exact right balance. Some questions are necessary such as what database are you going to use with Drizzle or else we can't setup an even basic integration. But we also want to keep the experience simple so that you get a basic setup without having to think too hard.

I can see the argument here either way. On the one hand you will need to setup the languages eventually. On the other hand, if you're just beginning a new application you might want it to be only in a single language for quite awhile until you expand into new markets. Setting up i18n from the beginning ensures you put your text into translatable strings so that when the time comes you can easily expand to a second language. But I think it's probably less common that an app starts with multiple languages from the beginning and so that's where just defaulting to English rather than asking for a list of languages might make setup simpler.

I suppose things that might be a bit more optional such as which paraglide languages to setup, which tailwind plugins to add, whether to add a dockerfile for your database could be moved into a sort of post-install set of questions. Things that are really necessary for a bare bones integration such as which database to use with Drizzle could remain in the main flow. I think we could play around with this separately though as it's a larger question affecting all adders, so I wouldn't block this PR on it.

delegating to a config tool of the added item ensures that this stays decoupled and you are in control of your own ui/dx

That doesn't sound like a benefit to me because it means the UX will be different for every adder. In any case, this isn't how any of the adders work today and calling out to a CLI for each tool seems like a separate discussion.

const { invalidLanguageTags, validLanguageTags } = parseLanguageTagInput(input);

if (invalidLanguageTags.length > 0)
return `Please enter a series of valid BCP47 language tags. Eg: en, de-ch, ar`;
Copy link

Choose a reason for hiding this comment

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

if this question is kept, this message should print the codes that are invalid so the user knows where the error is..

@benmccann
Copy link
Collaborator

@LorisSigrist was there any reason you had this marked as draft still? Let us know if you still have questions for us, have stuff you want to finish up, or are ready for us to review and possibly merge this. Thanks!

@samuelstroschein
Copy link

Hi @benmccann, I think this is good to go. I'll ask Loris if something is missing.

PS @LorisSigrist went back to university to continue his studies. I am interim taking over Paraglide JS again but resources are limited atm.

@samuelstroschein
Copy link

@benmccann loris told me that the PR is good to go if you are fine with adding paraglide js to svelte add. he mentioned that there were discussions to have a separate paraglide js init/external thing.

if you're fine with adding paraglide to svelte add, i think this can be merged.

@manuel3108 manuel3108 mentioned this pull request Oct 6, 2024
4 tasks
@manuel3108
Copy link
Member

Closing, so we can work on this in private. Will be release shortly.

@manuel3108 manuel3108 closed this Oct 6, 2024
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.

[Request] Add I18n library
5 participants
0