MINOR: Introduce new proxy implementation version #32
Conversation
src/proxyImplementationTemplate.ts
Outdated
| return ${useTranslateFn ? `t(prevKeys, ...argArray)` : 'prevKeys'}; | ||
| }, | ||
| });`; | ||
| } |
There was a problem hiding this comment.
This is exactly what I've did recently with data-hooks in settings 🤯
There was a problem hiding this comment.
I actually stole this approach from this package 😅
There was a problem hiding this comment.
Same 😄 .. I just wanted to make it support nesting, and fit our dataHooks approach
src/proxyImplementationTemplate.ts
Outdated
| ownValueAlias: string; | ||
| useTranslateFn: boolean; | ||
| }) { | ||
| return `const ${creatorFnName} = <R extends string>(t: (...args: unknown[]) => R = (k) => k as R, prevKeys: string = ''): unknown => |
There was a problem hiding this comment.
are you adding the return type of localeKeys and also the translations in english?
There was a problem hiding this comment.
im not sure im getting ur question
moreover I did refactor implementation and covered it with tests
could u plz take a look at this piece again? or we can catchap directly
| }); | ||
|
|
||
| export function LocaleKeys<R extends string>(t: (...args: unknown[]) => R) { | ||
| return createProxyImpl(t) as ILocaleKeys; |
There was a problem hiding this comment.
that is what i was wondering.. perfect!
src/bin.ts
Outdated
| @@ -72,6 +72,12 @@ const cliDefinition = yargs(hideBin(process.argv)).command( | |||
| type: 'boolean', | |||
| describe: 'Generate React bindings (Provider and hook)', | |||
| }) | |||
| .option('experimental_proxyImpl', { | |||
There was a problem hiding this comment.
is the plan to add it as an option, or eventually just replaced the approach from big object to the proxy approach?
I think it won't be breaking changes, and you should make it the only option.
can you think of any reason it will break for consumers?
There was a problem hiding this comment.
plan was to eventually replace to proxy as default
I played safe with the flag
but in general I'm also good with replacing it and having it enabled by default (bundlu reducing boost right away on new version!)
I will remove flag and enable this by default 👌
tkvlnk
changed the title
|
@varzager also updated docs and changelog |
| order: { | ||
| shippingLabel: { | ||
| customerDetailsCard: { | ||
| address: (data: Record<'firstName' | 'lastName' | 'addressLine1' | 'addressLine2' | 'city' | 'state' | 'zipCode' | 'country', unknown>) => t('order.shippingLabel.customerDetailsCard.address', data), /* {{firstName}} {{lastName}}, {{addressLine1, suffix ', '}}{{addressLine2, suffix ', '}}{{city, concat ', '}}{{state, suffix ' '}}{{zipCode, suffix ', '}}{{country}} */ | ||
| address: (data: Record<'firstName' | 'lastName' | 'addressLine1' | 'addressLine2' | 'city' | 'state' | 'zipCode' | 'country', unknown>) => string, /* {{firstName}} {{lastName}}, {{addressLine1, suffix ', '}}{{addressLine2, suffix ', '}}{{city, concat ', '}}{{state, suffix ' '}}{{zipCode, suffix ', '}}{{country}} */ |
There was a problem hiding this comment.
I have one concern!
and maybe it's only me doing it this way...
many times I search the content in localeKeys file and easily copy the their path from the actual function which makes it really easy while developing:
t('order.shippingLabel.customerDetailsCard.address', data)
now with the new approach this won't be possible.
option 1: It's not really important
option 2: we can add it as a comment before the value
It is just something i thought about, wanted to hear from you if you can relate, or we can just live without it
There was a problem hiding this comment.
option 2: we can add it as a comment before the value
sounds good to me
will do
Motivation:
Current generated files are huge and in fact mix into the bundle duplication of the messages file
So I got an idea to use a trick with typed Proxy object to be used in runtime
and the huge part of the file would be a typescript interface which would be stripped out on bundling