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

feat: translation #166

Merged
merged 9 commits into from
Feb 12, 2024
Merged

feat: translation #166

merged 9 commits into from
Feb 12, 2024

Conversation

tomasciccola
Copy link
Contributor

@tomasciccola tomasciccola commented Feb 6, 2024

should close #165

@tomasciccola
Copy link
Contributor Author

wondering how to validate languageCode. I know js has Intl.Locale but it seems you can pass some arbitrary stuff and it won't throw. I found this this library last updated 9 years ago that doesn't seem to do what it claims (return null on a valid bcp47 string)

@tomasciccola
Copy link
Contributor Author

tomasciccola commented Feb 6, 2024

another nice to have would be to validate recordType (check if the schema exists) and fieldRef, by traversing the array and checking if the provided field exists for that recordType.
validating recordId I don't think its easy since it may point to a record we don't know it exists yet?

@tomasciccola tomasciccola self-assigned this Feb 7, 2024
@tomasciccola
Copy link
Contributor Author

tomasciccola commented Feb 7, 2024

Another thing to address is how to solve a field ref for a field that is in an Array. i.e.
The options field in the Field record type is an array of {label:string, value: tagValue}
A fieldRef pointing to that would be smth like ['options','0', 'label'] ? meaning, 'this points to the 'label' field in the first option of a field?? This of course wouldn't work because fieldRef is string[]

Tomás Ciccola added 2 commits February 7, 2024 13:33
this additionally adds the `is-language-code` dependency for validation
@tomasciccola
Copy link
Contributor Author

wondering how to validate languageCode. I know js has Intl.Locale but it seems you can pass some arbitrary stuff and it won't throw. I found this this library last updated 9 years ago that doesn't seem to do what it claims (return null on a valid bcp47 string)

I went ahead and installed is-language-code which seems supported and up to date, but it uses smth called iana for language codes, which I don't know if we want to...

@gmaclennan
Copy link
Member

I don't think we should validate when encoding / decoding. I think we should do that when writing. Invalid data will just be ignored, because lookups will just not work. If possible, we can add a type to recordType (Exclude<MapeoDoc['schemaName'], 'translation'>).

I wrote up thoughts on language codes in the issue.

For the fieldRef, I think we should just use a string, using dot-prop notation. This solves the question about field options (the example would be options[0].label). I don't think the encoding or decoding here should do anything with that prop though.

Tomás Ciccola added 2 commits February 8, 2024 13:49
1. `fieldRef` is now a `string`
2. remove validation of fields when encoding
3. Add good-doc-completed
Copy link
Member

@gmaclennan gmaclennan left a comment

Choose a reason for hiding this comment

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

Just missing the region code, plus some suggested copy edits. Otherwise good to merge I think.

package.json Outdated Show resolved Hide resolved
schema/translation/v1.json Outdated Show resolved Hide resolved
schema/translation/v1.json Outdated Show resolved Hide resolved
schema/translation/v1.json Outdated Show resolved Hide resolved
schema/translation/v1.json Outdated Show resolved Hide resolved
schema/translation/v1.json Outdated Show resolved Hide resolved
schema/translation/v1.json Show resolved Hide resolved
proto/translation/v1.proto Outdated Show resolved Hide resolved
tomasciccola and others added 4 commits February 12, 2024 12:37
Rename fields and change some descriptions from JSONSchema

Co-authored-by: Gregor MacLennan <[email protected]>
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.

Translation record type
2 participants