gestalt
gestalt copied to clipboard
I18nProvider: rework Flow types to accommodate expansion
We need to use I18nProvider for more components, but Flow complains about the union types created when adding additional components. This is ultimately a limit of static analysis: while we provide a generic type C
for the component name provided to useI18nContext
, Flow cannot know what the runtime component will be statically. Therefore any references to propsTranslations[componentName]
must be a union of all possible I18nContextType
key fields, and iterating through them (e.g. the .filter
call) while attempting to access componentTranslations
properties must accommodate all possible keys, not just those for the current component.
Consider this code:
const translationPropNames = Object.keys(componentTranslations);
const nullTranslations = translationPropNames.filter(
(propName) => typeof componentTranslations[propName] !== 'string',
);
Flow complains because propName
is inferred to be "accessibilityClearButtonLabel" | "accessibilityHidePasswordLabel" | "accessibilityShowPasswordLabel"
, and while Flow doesn't know which component object we'll be looking at, it knows that no object will have all of those fields.
One possible solution is to make the data structure a bit weaker:
type I18nContextType = {|
ComboBox: {|
accessibilityClearButtonLabel: ?string,
[string]: ?string,
|},
TextField: {|
accessibilityHidePasswordLabel: ?string,
accessibilityShowPasswordLabel: ?string,
[string]: ?string,
|},
|};
This isn't great. While any extra fields will be ignored within components, DX suffers as we're no longer able to let the user know if they have a typo or are providing a field that isn't yet supported.
Another solution is to cast the type of propName
:
const translationPropNames = Object.keys(componentTranslations);
const nullTranslations = translationPropNames.filter(
(propName: string) => typeof componentTranslations[propName] !== 'string',
);
This is fine. We're telling Flow not to try too hard as propName
can be any string. This seems a bit unsafe, and theoretically it is — but this is inherent in how Flow treats objects as maps:
When an object type has an indexer property, property accesses are assumed to have the annotated type, even if the object does not have a value in that slot at runtime. It is the programmer’s responsibility to ensure the access is safe, as with arrays.
However, these checks were a bit belt-and-suspenders anyway — Flow already provides the safety we need here, in that attempting to use I18nProvider without providing translations will already throw a Flow error. As with the previous approach, we can improve this DX in dev by adding a more explicit error message (also checking for empty strings) and letting the dev know which component is missing translations. (We can no longer say which exact translations are missing, but this isn't important.)
Of note is that we're no longer providing default (English) strings. I never wanted to include those in the first place, but couldn't figure out the proper typing with my initial Pinboard implementation, so had to tack them on. Using English default strings too easily covers up missing translations and buries those errors.
Deploy Preview for gestalt ready!
Name | Link |
---|---|
Latest commit | cfa74b13a42343a9ae39ecbbf5d3d8fb201e8f7b |
Latest deploy log | https://app.netlify.com/sites/gestalt/deploys/633f7682dd966e0008e1e848 |
Deploy Preview | https://deploy-preview-2407--gestalt.netlify.app |
Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site settings.