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

split language negotiation settings into detection and storage #1221

Closed
ivanhofer opened this issue Aug 9, 2023 · 21 comments
Closed

split language negotiation settings into detection and storage #1221

ivanhofer opened this issue Aug 9, 2023 · 21 comments
Assignees
Labels
scope: inlang/paraglide-js Related to source-code/sdk-js. type: improvement Something that can be improved.

Comments

@ivanhofer
Copy link
Contributor

It is not so easy to know how to configure the strategies array in the sdk settings.

There are multiple options someone can choose from and they slightly differ from each other. Some strategies are read-only because the application cannot update them and some of them can be altered.

read-only:

  • navigator
  • acceptLanguageHeader

others:

  • url
  • cookie
  • localStorage
  • sessionStorage

Other strategies can be added in the future, but they will probably fall into one of those two categories.

To make this more clear we should split them into detection and storage (or find some better words).

@ivanhofer ivanhofer converted this from a draft issue Aug 9, 2023
@ivanhofer ivanhofer self-assigned this Aug 9, 2023
@maige-app maige-app bot added type: improvement Something that can be improved. scope: inlang/paraglide-js Related to source-code/sdk-js. labels Aug 9, 2023
@ivanhofer ivanhofer removed the type: improvement Something that can be improved. label Aug 9, 2023
@ivanhofer
Copy link
Contributor Author

This should happen after the inlang core refactoring to be able to update the config automatically

@ivanhofer
Copy link
Contributor Author

url could also be used as a read-only strategy e.g. a website uses ?lang="de" to redirect a user to the german version of the website, but from there on the language should be stored in a cookie

@ivanhofer
Copy link
Contributor Author

A user could not set any strategy if he wants to handle it manually. We then need to make sure that setLanguageTag get's called

  • on the server during handle before resolve(event) or
  • client side during load in the root +layout.js file. In this case we need to delay all child load functions to make sure that the runtime was initialized.

@ivanhofer
Copy link
Contributor Author

We should make storage: [{ type: 'url' }] the default and throw an error when the [lang] slug is missing. The error should make clear what to do and how to change the config if another language detection option should be used.

@ivanhofer
Copy link
Contributor Author

We should show a warining if the configured options do not make sense e.g. localStorage with SSR or cookie with static output. Or even throw an error.

@ivanhofer
Copy link
Contributor Author

ivanhofer commented Aug 9, 2023

The API could look like this:

type Config = {
	detection: 'auto' | 'server' | 'client' | 'off'
	storage: Strategy[]
}

type Strategy = { type: 'url' | 'localStorage' | 'cookie' /* ... */ }
  • server and client are aliases for acceptLanguageHeader and navigator to make it easier for beginners to set it up
  • detection: 'auto' is the default and it looks through the storage options to find out if server or client should be chosen
  • when setting detection: 'off' developers opt out of that process and they need to manually call setLanguageTag somewhere

If any combination does not make sense, we throw an error.

The default config is:

{
	detection: 'auto',
	storage: [{ type: 'url' }]
}

@ivanhofer ivanhofer added effort: medium type: improvement Something that can be improved. labels Aug 9, 2023
@ivanhofer
Copy link
Contributor Author

Does it now still make sense to have storage defined as an Array?

Probably not and it would make things harder to explain. If the languageTag cannot be retrieved from the storage option, it means that someone has not used the app before and detection should kick in.
The only use case I see is when migrating from e.g. cookie to url. Then it could make sense to have an array to make the migration without users noticing it. But in this case a developer can also write a manual function to do that.

Conclusion: get rid of the Array

@samuelstroschein
Copy link
Member

samuelstroschein commented Aug 9, 2023

Question: Does the SDK need so many options if the SDK is designed for best practices?

For example, how to do language detection is surely defined as best practice by W3C. Same for how to persist the user's language. Should we even give options? Shouldn't we foster best practices?

@ivanhofer
Copy link
Contributor Author

If we want to design a SDK that everyone uses, we need to provide some options. This API design does not mean we need to implement it. It just makes sure we can easily extend it in the future. The options we need for sure are currently defined here. The commented out options are nice to have for the future.

@samuelstroschein
Copy link
Member

samuelstroschein commented Aug 9, 2023

If we want to design a SDK that everyone uses, we need to provide some options.

After the refactoring, we should formulate design principles.

I think we should design for the 80% use cases and foster best practices. We should rather make 80% of potential users really happy than 90% of users "okay happy" because too much config, still doesn't support edge cases, etc. The rest can build their own SDKs based on @inlang/sdk-js-core.

Anyhow, let's finish the refactoring first.

@ivanhofer
Copy link
Contributor Author

Please tell me how we can support classical SPA and static SSR applications without those options?

@samuelstroschein
Copy link
Member

samuelstroschein commented Aug 9, 2023

Please tell me how we can support classical SPA and static SSR applications without those options?

It seems clear that one sdk-js config to rule them all might be the wrong path. Because setting up the SDK-js will become complicated. For example, as a react SPA user I don't care about SvelteKit/metaframework config stuff and yet the docs will mention them and overload the user.

Some ideas:

  1. Split the sdk-js into sdk-js, sdk-js-{framework}, and sdk-js-{metaframework}.
  2. Make the SDK-JS itself pluggable. The new appSpecificApi makes a pluggable sdk-js possible.

An example of a pluggable SDK-JS for a react user:

modules: ["./inlang-sdk-plugin-react.js"],
settings: {
  plugins: {
    "inlang.sdk-plugin-react": {
      options: {
        // ... react specific options
      },
    },
  },
},

For a sveltekit user:

modules: ["./inlang-sdk-plugin-sveltekit.js"],
settings: {
  plugins: {
    "inlang.sdk-plugin-sveltekit": {
      options: {
        // ... sveltekit specific options
      },
    },
  },
},

Pros

  • one import to rule them all remains @inlang/sdk-js
  • pick and choose your plugin/framework (easy communication)
  • don't get overloaded by config options that don't concern your plugin/framework

Cons

  • @inlang/sdk-js-react etc. is more familiar to developers

@samuelstroschein
Copy link
Member

Another note, the detections and storage options are obsolete in a server context like our CLI or a classical web server. Bundling the SDK-JS with those metaframework-specific things is likely undesired for the sake of keeping dependencies simple.

Which raises another design principle question: Aren't metaframeworks just a feature? Why are we designing the SDK-JS around a feature instead of building the SDK-JS out as a platform that has a good metaframework feature?

@ivanhofer
Copy link
Contributor Author

In my opinion the detection and storage keys would still make sense to all applications. Just with slightly different options.

The arguments that now come up are not related to the original issue, but how to structure the settings.

For adapter specific settings I was thinking about adding a prop to the top level of the sdk settings like this:

"settings": {
	"plugins": {
		"inlang.sdk-js": {
			"options": {
				"sveltekit": {
					// SvelteKit specific options
				},
				// all shared options
				"resources": { }
			},
		},
	},
},

This has the benefit of having a single config serving multiple applications without needing to specify everything multiple times.

But since we now (finally) have decided to have dedicated packages for each adapter, we can think of splitting the settings also into dedicated plugins.

@samuelstroschein

This comment was marked as off-topic.

@samuelstroschein

This comment was marked as off-topic.

@ivanhofer

This comment was marked as off-topic.

@samuelstroschein

This comment was marked as off-topic.

@dominikg
Copy link

dominikg commented Aug 25, 2023

How exactly language is determined/stored is very application specific. It makes a lot of sense to offer different algorithms and configuration to pick them. I'd also like to see ways to completely customize it and/or combine different ones.

It's also different between ssr pages and spa. With ssr, the app should render <html lang="xx"> where xx is the result of language determination algoritm and then the client can just pick up that result by parsing the lang attribute.

@dominikg
Copy link

some example code sveltejs/kit#6183 (comment)

https://github.com/dominikg/kit-with-svelte-intl-precompile (outdated kit version but same stuff still applies today)

@jannesblobel
Copy link
Member

Unfortunately, Ivan has passed away, and we are trying to find a solution.
I will close this issue as not planned for now when we have a solution.

@jannesblobel jannesblobel closed this as not planned Won't fix, can't repro, duplicate, stale Sep 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: inlang/paraglide-js Related to source-code/sdk-js. type: improvement Something that can be improved.
Projects
No open projects
Development

No branches or pull requests

4 participants