-
Notifications
You must be signed in to change notification settings - Fork 39
Conversation
🦋 Changeset detectedLatest 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 |
adders/paraglide/config/adder.ts
Outdated
// sourceLanguageTag: 'en', | ||
// languageTags: ['en'], | ||
modules: [ | ||
'https://cdn.jsdelivr.net/npm/@inlang/message-lint-rule-empty-pattern@latest/dist/index.js', |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Totally agree!
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
commit: |
There was a problem hiding this 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
adders/paraglide/config/adder.ts
Outdated
// sourceLanguageTag: 'en', | ||
// languageTags: ['en'], | ||
modules: [ | ||
'https://cdn.jsdelivr.net/npm/@inlang/message-lint-rule-empty-pattern@latest/dist/index.js', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Totally agree!
|
||
export const options = defineAdderOptions({ | ||
availableLanguageTags: { | ||
question: 'Which language tags would you like to support?', |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
adders/paraglide/config/options.ts
Outdated
const { invalidLanguageTags, validLanguageTags } = parseLanguageTagInput(input); | ||
|
||
if (invalidLanguageTags.length > 0) | ||
return `Please enter a series of valid BCP47 language tags. Eg: en, de-ch, ar`; |
There was a problem hiding this comment.
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..
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
@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! |
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. |
@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. |
Closing, so we can work on this in private. Will be release shortly. |
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