I feel that this new feature can be fully tested with unit tests only and I don't want to overload the e2e test run. But let me know if otherwise; it's a very small matter to add an e2e test if needed.
Speaking of tests, I also had to switch some of the unit tests to serial execution to make them pass. The reason is that, with the current implementation of the I18n extension as a singleton, parallel access is prone to race conditions (I'll submit a proposal shortly for fixing that).
And lastly, I'm not sure of which TS types would fit best (I'm not familiar with best practices, as I don't code in TypeScript myself). I simply added ... | null | undefined to the previous string types. This keeps disturbance of the previous definitions to a minimum while highlighting that null and undefined now have a special meaning. But, since the new implementation of setLocale(locale) effectively converts its argument to a string, should those types be any instead?
I feel that this new feature can be fully tested with unit tests only and I don't want to overload the e2e test run. But let me know if otherwise; it's a very small matter to add an e2e test if needed.
Speaking of tests, I also had to switch some of the unit tests to serial execution to make them pass. The reason is that, with the current implementation of the I18n extension as a singleton, parallel access is prone to race conditions (I'll submit a proposal shortly for fixing that).
And lastly, I'm not sure of which TS types would fit best (I'm not familiar with best practices, as I don't code in TypeScript myself). I simply added
... | null | undefined
to the previousstring
types. This keeps disturbance of the previous definitions to a minimum while highlighting thatnull
andundefined
now have a special meaning. But, since the new implementation ofsetLocale(locale)
effectively converts its argument to a string, should those types beany
instead?(this PR implements proposal #55)