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

Cannot extract using i18n.plural : undefined is not iterable! #1022

Closed
kopax-polyconseil opened this issue Apr 2, 2021 · 74 comments
Closed

Comments

@kopax-polyconseil
Copy link

kopax-polyconseil commented Apr 2, 2021

Hello, we use lingui v2.9.2 and we got issue extracting using i18n.plural:

This works:

export function _(
  id: MessageDescriptor | string,
  values?: Record<string, unknown>,
  messageOptions?: MessageOptions
): string {
  if (typeof id === 'string') {
    return i18n._(id, values, messageOptions)
  }
  return i18n._(id)
}

This fail:

export function _(id: MessageDescriptor | PluralProps): string {
  return !Object.prototype.hasOwnProperty.call(id, 'one')
    ? i18n._(id as MessageDescriptor)
    : i18n.plural(id as PluralProps)
}

It fails when we run lingui extract --clean, with the following error:

TypeError: src/libs/i18n.ts: undefined is not iterable!
    at module.exports.require.getIterator (/home/dka/workspace/github.com/pass-culture/pass-culture-app-native/node_modules/babel-runtime/node_modules/core-js/library/modules/core.get-iterator.js:5:42)
    at Transformer.transformChoiceMethod (/home/dka/workspace/github.com/pass-culture/pass-culture-app-native/node_modules/@lingui/babel-plugin-transform-js/transformer.js:199:56)
    at Transformer.transformMethod (/home/dka/workspace/github.com/pass-culture/pass-culture-app-native/node_modules/@lingui/babel-plugin-transform-js/transformer.js:393:21)
    at PluginPass.Transformer.transform (/home/dka/workspace/github.com/pass-culture/pass-culture-app-native/node_modules/@lingui/babel-plugin-transform-js/transformer.js:69:25)
    at newFn (/home/dka/workspace/github.com/pass-culture/pass-culture-app-native/node_modules/babel-traverse/lib/visitors.js:276:21)
    at NodePath._call (/home/dka/workspace/github.com/pass-culture/pass-culture-app-native/node_modules/babel-traverse/lib/path/context.js:76:18)
    at NodePath.call (/home/dka/workspace/github.com/pass-culture/pass-culture-app-native/node_modules/babel-traverse/lib/path/context.js:48:17)
    at NodePath.visit (/home/dka/workspace/github.com/pass-culture/pass-culture-app-native/node_modules/babel-traverse/lib/path/context.js:105:12)
    at TraversalContext.visitQueue (/home/dka/workspace/github.com/pass-culture/pass-culture-app-native/node_modules/babel-traverse/lib/context.js:150:16)
    at TraversalContext.visitSingle (/home/dka/workspace/github.com/pass-culture/pass-culture-app-native/node_modules/babel-traverse/lib/context.js:108:19) {
  _babel: true
}

We rather use the synthax of i18n.plural instead of i18n._, how can this be fixed ?

Reproduction

git clone https://github.com/pass-culture/pass-culture-app-native.git
cd pass-culture-app-native
nvm use
yarn
yarn run translations:extract

Related issues

@semoal
Copy link
Contributor

semoal commented Apr 2, 2021

Hey mate, i could take a look to the issue. But... what do you think if you give some days and i migrate your entire codebase to Lingui v3 with the best practices?
Is it possible for your side (to allow me touch your codebase, you know)?

@kopax
Copy link

kopax commented Apr 2, 2021

HI @semoal , I am delighted of your proposition and of course, I can only accept if that solves our problem. I tried to migrate the whole today but the step were not straightforward so I rolled back.

You can fork and create a repo, I'll reproduce the PR when needed.

@semoal
Copy link
Contributor

semoal commented Apr 2, 2021

I took a look and looks there's no much to change and I'm sure that for the future will be easier for you, with all the power of Lingui with plurals, select and other cool methods.

(Of course will be a good real example how a migration to v3 can be done and useful for documenting or fixing any that is wrong on v3)

I'll create a fork this weekend and probably in a few days I'll create a pr! 👍🏻

@semoal
Copy link
Contributor

semoal commented Apr 6, 2021

I took a look and looks there's no much to change and I'm sure that for the future will be easier for you, with all the power of Lingui with plurals, select and other cool methods.

(Of course will be a good real example how a migration to v3 can be done and useful for documenting or fixing any that is wrong on v3)

I'll create a fork this weekend and probably in a few days I'll create a pr! 👍🏻

Along today or tomorrow I'll create the pull request mate, I'm polishing some details :)

I'm already green our your entire suite =)

Test Suites: 222 passed, 222 total
Tests:       4 skipped, 1271 passed, 1275 total
Snapshots:   88 passed, 88 total
Time:        75.51s
Ran all test suites.
✨  Done in 76.80s.

@kopax-polyconseil
Copy link
Author

Thanks @semoal that's great, did you manage to use i18n.plural API? May I have a check on your branch?

@semoal
Copy link
Contributor

semoal commented Apr 6, 2021

Thanks @semoal that's great, did you manage to use i18n.plural API? May I have a check on your branch?

Yes, just tried to use the macro and the plural() macro and both works nicely :)

.tsx file somehwere there:

      <Plural value={0} one="# prueba" other="# pruebas" />
      {plural(0, {
        one: '# message',
        other: '# messages',
      })}

.po extracted:

#: src/features/bookings/components/BookingPropertiesSection.tsx:20
msgid "{0, plural, one {# message} other {# messages}}"
msgstr "{0, plural, one {# message} other {# messages}}"

#: src/features/bookings/components/BookingPropertiesSection.tsx:19
msgid "{0, plural, one {# prueba} other {# pruebas}}"
msgstr "{0, plural, one {# prueba} other {# pruebas}}"

@semoal
Copy link
Contributor

semoal commented Apr 6, 2021

Here is the pull request @kopax pass-culture/pass-culture-app-native#842
I couldn't start the application because crashes my ios installation (i have some other applications an probably are colliding), so give it a try and tell me if everything works good :)

@kopax-polyconseil
Copy link
Author

@semoal, I just rebased and tested your PR: and it seems the syntax in i18n.ts is not valid:

import { i18n } from '@lingui/core'
import { fr } from 'make-plural/plurals'

i18n.loadLocaleData({
  fr: { plurals: fr },
})

export const activate = async (locale: string) => {
  const { messages } = await import(`../locales/${locale}/messages`)
  i18n.load(locale, messages)
  i18n.activate(locale)
}

Result on startup error:

error: src/libs/i18n.ts: src/libs/i18n.ts:Invalid call at line 9: import("../locales/" + locale + "/messages")

Do you have a clue? I am now investigating.

@semoal
Copy link
Contributor

semoal commented Apr 7, 2021

@semoal, I just rebased and tested your PR: and it seems the syntax in i18n.ts is not valid:

import { i18n } from '@lingui/core'
import { fr } from 'make-plural/plurals'

i18n.loadLocaleData({
  fr: { plurals: fr },
})

export const activate = async (locale: string) => {
  const { messages } = await import(`../locales/${locale}/messages`)
  i18n.load(locale, messages)
  i18n.activate(locale)
}

Result on startup error:

error: src/libs/i18n.ts: src/libs/i18n.ts:Invalid call at line 9: import("../locales/" + locale + "/messages")

Do you have a clue? I am now investigating.

Yes, that's my bad (not an expert react-native developer, just web ^^) looks metro/native/expo i don't know how it's called doesn't support dynamic imports
facebook/metro#52 (comment)

you can reimplement this to:

export async function activate(locale: string) {
   let catalog: {messages: Messages}

   switch (locale) {
     case 'en':
       catalog = await import('./locales/en/messages.po')
       break
     case 'fr':
     default:
       catalog = await import('./locales/fr/messages.po')
       break;
   }

   i18n.load(locale, catalog.messages)
   i18n.activate(locale)
}

Feel free to ping me if you need anything else by my side, Lingui questions or whenever you need =)

@kopax-polyconseil
Copy link
Author

@semoal thanks again for your support, so far, I have removed the import the dynamic import part and there's an issue with make-plurals:

error: Error: Unable to resolve module `make-plural/plurals` from `src/libs/i18n.ts`: make-plural/plurals could not be found within the project.

I've checked and followed the recommendation to clean the projet, despite the module is existing in node_modules, I can't get my app to start, if you have a clue, thanks, otherwise I'll keep digging.

@semoal
Copy link
Contributor

semoal commented Apr 7, 2021

@semoal thanks again for your support, so far, I have removed the import the dynamic import part and there's an issue with make-plurals:

error: Error: Unable to resolve module `make-plural/plurals` from `src/libs/i18n.ts`: make-plural/plurals could not be found within the project.

I've checked and followed the recommendation to clean the projet, despite the module is existing in node_modules, I can't get my app to start, if you have a clue, thanks, otherwise I'll keep digging.

eemeli/make-plural#15
Looks like you have to accept your metro config to accept .mjs files, since make-plural resolves to browser instead (also make sure that is installed, i'm not sure about that haha)

@kopax-polyconseil
Copy link
Author

kopax-polyconseil commented Apr 7, 2021

OK, so far I also disabled the import for plurals and now we got this:

Plurals for locale fr aren't loaded. Use i18n.loadLocaleData method to load plurals for specific locale. Using other plural rule as a fallback. 

image

@semoal
Copy link
Contributor

semoal commented Apr 7, 2021

Actually loading the plurals is required, so the loadLocaleData is mandatory :/

@kopax-polyconseil
Copy link
Author

kopax-polyconseil commented Apr 7, 2021

Yes, I just succeded to load the plural editing my metro.config.js with :

/**
 * Metro configuration for React Native
 * https://github.com/facebook/react-native
 *
 * @format
 */

module.exports = {
+  resolver: {
+    sourceExts: ['js', 'ts', 'tsx', 'mjs'],
+  },
  transformer: {
    getTransformOptions: async () => ({
      transform: {
        experimentalImportSupport: false,
        inlineRequires: false,
      },
    }),
  },
  maxWorkers: 2,
}

I still have the error from the screenshot.

That's how my i18n.ts now look like:

import { i18n, Messages } from '@lingui/core'
import { fr } from 'make-plural/plurals'

i18n.loadLocaleData({
  fr: { plurals: fr },
})

export async function activate(locale: string) {
  let catalog: { messages: Messages }

  switch (locale) {
    case 'fr':
    default:
      catalog = await import('locales/fr/messages')
      break
  }

  i18n.load(locale, catalog.messages)
  i18n.activate(locale)
}

Edit

Interesting, in my code, if I replace:

- text={t`dans l’année de tes 18 ans, obtiens l’aide financière pass Culture d’un montant de ${deposit} à dépenser dans l’application.`}
+ text={t`dans l’année de tes 18 ans, obtiens l’aide financière pass Culture d’un montant de à dépenser dans l’application.`}

It seems to be an issue with template literals.

@kopax-polyconseil
Copy link
Author

I figured the cause, it seems that now undefined cannot be trimed(), so we cannot use undefined variable in template literrals, this cause most of the page of our application to crash. Is there a soft mode or something we can use ?

@semoal
Copy link
Contributor

semoal commented Apr 7, 2021

Let me hotfix this 👍🏻 , try to patch the package and check if we need to change anything else:

var result = formatMessage(translation);
    return result && result.trim();

Already created the pr: #1030 , I'll fix anything you find and i'll release a new version in the same moment :)

@kopax-polyconseil
Copy link
Author

kopax-polyconseil commented Apr 7, 2021

image

I added some logging to see what value is result...

Edit

And log shows:

LOG      [Function dansLAnnEDeTes18AnsObtiensLAideFinanciRePassCultureDUnMontantDeDepositDPenserDansLApplication]

@kopax-polyconseil
Copy link
Author

kopax-polyconseil commented Apr 7, 2021

This seems to work: return result && result.trim && result.trim();,

Why is result sometimes a function? Why would you want to call trim on a function?

@semoal
Copy link
Contributor

semoal commented Apr 7, 2021

This seems to work: return result && result.trim && result.trim();,

Why is result sometimes a function? Why would you want to call trim on a function?

Result shouldn't be a function because are the messages interpolated, I'm checking if i can reproduce the issue in our test suite

@kopax-polyconseil
Copy link
Author

  • Also i saw that you use a lot unicode chars to give spaces to things, in Lingui v3 we strip this characters so just use html entities or basic templating

I still see a lot of unicode chars displayed, I have tried to remove them, using some HTML code. Can you tell me how for example I can replace \u00a0 ?

@semoal
Copy link
Contributor

semoal commented Apr 7, 2021

  • Also i saw that you use a lot unicode chars to give spaces to things, in Lingui v3 we strip this characters so just use html entities or basic templating

I still see a lot of unicode chars displayed, I have tried to remove them, using some HTML code. Can you tell me how for example I can replace \u00a0 ?

Just an empty space should be enough if not, you can use perfectly the equivalent in html code entities &nbsp; but i'm not sure if that will work with react-native since there's no dom.

@kopax-polyconseil
Copy link
Author

kopax-polyconseil commented Apr 7, 2021

I've tried both and none of them work. Why would you strip the characters ? Perhaps you can add an option to toggle the feature for such cases?

@semoal
Copy link
Contributor

semoal commented Apr 7, 2021

I've tried both and none of them work. Why would you strip the characters ? Perhaps you can add an option to toggle the feature for such cases?

I can't remember now why Tomas took that direction, we had issues with some bundlers and we decided to keep it like just html entities.
What's the case that doesn't work?

t`Some translate `  + something appended?

In this case is recommended to use it like this:

t`Some translate ${something appended}`

offtopic could you try to typeof the previous issue to check if everything works good?:

    const result = formatMessage(translation)
    if (typeof result === "string") return result.trim()
    return result

@kopax-polyconseil
Copy link
Author

kopax-polyconseil commented Apr 7, 2021

We use syntax literal so it's a template literal that caused to be a function instead of a string:

[Wed Apr 07 2021 15:24:41.213]  LOG      [Function _]
[Wed Apr 07 2021 15:24:41.542]  LOG      [Function Version0]
[Wed Apr 07 2021 15:24:41.863]  LOG      [Function DSLe0]
[Wed Apr 07 2021 15:24:41.905]  LOG      [Function Version0]
[Wed Apr 07 2021 15:24:42.578]  LOG      [Function TuAs0SurTonPass]
[Wed Apr 07 2021 15:24:42.580]  LOG      [Function Bonjour0]
[Wed Apr 07 2021 15:24:42.582]  LOG      [Function DSLe0]
[Wed Apr 07 2021 15:24:42.623]  LOG      [Function Version0]
[Wed Apr 07 2021 15:24:42.852]  LOG      [Function TuAs0SurTonPass]
[Wed Apr 07 2021 15:24:42.873]  LOG      [Function Bonjour0]
[Wed Apr 07 2021 15:24:42.875]  LOG      [Function DSLe0]
[Wed Apr 07 2021 15:24:43.480]  LOG      [Function _]
[Wed Apr 07 2021 15:24:43.187]  LOG      [Function TuAs0SurTonPass]
[Wed Apr 07 2021 15:24:43.189]  LOG      [Function Bonjour0]

I've updated the interpolate and it work with the updated snippet.

I can't remember now why Tomas took that direction, we had issues with some bundlers and we decided to keep it like just html entities.

We use a lot of unicode characteres to add unbreakable spaces, etc... Can this feature be restored? Otherwise, how do you recommend to solve it?

@semoal
Copy link
Contributor

semoal commented Apr 7, 2021

Let me review the unicode topic

@semoal
Copy link
Contributor

semoal commented Apr 7, 2021

Give me tonight to analyse how can we solve the unicode part and tomorrow we conclude this ;)

@kopax-polyconseil
Copy link
Author

Alright, thanks again for helping us in the migration process ! Good luck,

@kopax-polyconseil
Copy link
Author

It's all merged. I'll close it for now and re-open it if needed. Thanks again for helping us migrating the library, that's really cool.

@kopax-polyconseil
Copy link
Author

kopax-polyconseil commented Apr 8, 2021

We have another issue here:

https://github.com/pass-culture/pass-culture-app-native/blob/4bc0d48d0d51cba8332e706eb1ffdd1a195ab602/src/features/firstTutorial/pages/FirstTutorial/components/SecondCard.tsx#L27

Despite amount value being 500€ here, it gives me the following logs when I check the value:

[Thu Apr 08 2021 14:50:17.425]  LOG      300€
[Thu Apr 08 2021 14:50:17.469]  ERROR    Warning: Functions are not valid as a React child. This may happen if you return a Component instead of <Component /> from render. Or maybe you meant to call this function rather than return it.
    in RCTText (at Text.js:159)
    in TouchableText (at Text.js:283)
    in Text (created by Context.Consumer)
    in StyledNativeComponent (created by Styled(Styled(Styled(Text))))
    in Styled(Styled(Styled(Text))) (at GenericAchievementCard.tsx:102)
    in RCTView (at View.js:34)
    in View (created by Context.Consumer)
    in StyledNativeComponent (created by Styled(View))
    in Styled(View) (at GenericAchievementCard.tsx:88)
    in GenericAchievementCard (at SecondCard.tsx:26)
    in SecondCard (at FirstTutorial.tsx:19)
    in RCTView (at View.js:34)
    in View (created by Swiper)
    in RCTView (at View.js:34)
    in View (at createAnimatedComponent.js:165)
    in AnimatedComponent (at createAnimatedComponent.js:215)
    in ForwardRef(AnimatedComponentWrapper) (created by Swiper)
    in RCTView (at View.js:34)
    in View (created by Swiper)
    in RCTView (at View.js:34)
    in View (created by Swiper)
    in Swiper (at GenericAchievement.tsx:61)
    in RCTView (at View.js:34)
    in View (created by Context.Consumer)
    in StyledNativeComponent (created by Styled(View))
    in Styled(View) (at GenericAchievement.tsx:60)
    in RCTView (at View.js:34)
    in View (created by Context.Consumer)
    in StyledNativeComponent (created by Styled(View))
    in Styled(View) (at GenericAchievement.tsx:52)
    in RCTView (at View.js:34)
    in View (created by Context.Consumer)
    in StyledNativeComponent (created by Styled(View))
    in Styled(View) (at GenericAchievement.tsx:50)
    in GenericAchievement (at FirstTutorial.tsx:17)
    in FirstTutorial (at SceneView.tsx:122)
    in StaticContainer
    in StaticContainer (at SceneView.tsx:115)
    in EnsureSingleNavigator (at SceneView.tsx:114)
    in SceneView (at useDescriptors.tsx:153)
    in RCTView (at View.js:34)
    in View (at CardContainer.tsx:221)
    in RCTView (at View.js:34)
    in View (at CardContainer.tsx:220)
    in RCTView (at View.js:34)
    in View (at CardSheet.tsx:33)
    in ForwardRef(CardSheet) (at Card.tsx:563)
    in RCTView (at View.js:34)
    in View (at createAnimatedComponent.js:165)
    in AnimatedComponent (at createAnimatedComponent.js:215)
    in ForwardRef(AnimatedComponentWrapper) (at Card.tsx:545)
    in PanGestureHandler (at GestureHandlerNative.tsx:13)
    in PanGestureHandler (at Card.tsx:539)
    in RCTView (at View.js:34)
    in View (at createAnimatedComponent.js:165)
    in AnimatedComponent (at createAnimatedComponent.js:215)
    in ForwardRef(AnimatedComponentWrapper) (at Card.tsx:535)
    in RCTView (at View.js:34)
    in View (at Card.tsx:529)
    in Card (at CardContainer.tsx:189)
    in CardContainer (at CardStack.tsx:558)
    in RCTView (at View.js:34)
    in View (at Screens.tsx:69)
    in MaybeScreen (at CardStack.tsx:551)
    in RCTView (at View.js:34)
    in View (at Screens.tsx:48)
    in MaybeScreenContainer (at CardStack.tsx:461)
    in CardStack (at StackView.tsx:458)
    in KeyboardManager (at StackView.tsx:456)
    in SafeAreaProviderCompat (at StackView.tsx:453)
    in GestureHandlerRootView (at GestureHandlerRootView.android.js:31)
    in GestureHandlerRootView (at StackView.tsx:452)
    in StackView (at createStackNavigator.tsx:84)
    in StackNavigator (at RootNavigator.tsx:35)
    in RootNavigator (at App.tsx:71)
    in EnsureSingleNavigator (at BaseNavigationContainer.tsx:407)
    in ForwardRef(BaseNavigationContainer) (at NavigationContainer.tsx:91)
    in ThemeProvider (at NavigationContainer.tsx:90)
    in ForwardRef(NavigationContainer) (at NavigationContainer.tsx:23)
    in AppNavigationContainer (at App.tsx:70)
    in SplashScreenProvider (at App.tsx:69)
    in SnackBarProvider (at App.tsx:68)
    in I18nProvider (at App.tsx:67)
    in SearchWrapper (at App.tsx:66)
    in FavoritesWrapper (at App.tsx:65)
    in AuthWrapper (at App.tsx:64)
    in GeolocationWrapper (at App.tsx:63)
    in ErrorBoundary (at App.tsx:62)
    in QueryClientProvider (at App.tsx:61)
    in RNCSafeAreaProvider (at SafeAreaContext.tsx:74)
    in SafeAreaProvider (at App.tsx:60)
    in ABTestingProvider (at App.tsx:59)
    in App (at renderApplication.js:45)
    in RCTView (at View.js:34)
    in View (at AppContainer.js:106)
    in RCTView (at View.js:34)
    in View (at AppContainer.js:132)
    in AppContainer (at renderApplication.js:39)
[Thu Apr 08 2021 14:50:18.175]  LOG      300€
[Thu Apr 08 2021 14:50:18.471]  LOG      500€

The text is not displayed:

image

The following code:

      text={
        t`dans l’année de tes 18 ans, obtiens l’aide financière pass Culture d’un montant de\u00a0` +
        deposit +
        `\u00a0à dépenser dans l’application.`
      }

cause the following:

image

Adding log in the component that receive the text:

  • <FirstCard /> props text: [Thu Apr 08 2021 14:57:28.524] LOG une initiative financée par le Ministère de la Culture.
  • <SecondCard /> props text: [Thu Apr 08 2021 14:57:28.569] LOG function dansLAnnEDeTes18AnsObtiensLAideFinanciRePassCultureDUnMontantDeDepositDPenserDansLApplication(a) { return ["dans l\u2019ann\xE9e de tes 18 ans, obtiens l\u2019aide financi\xE8re pass Culture d\u2019un montant de ", a("deposit"), " \xE0 d\xE9penser dans l\u2019application."]; }

It seems that the macro is having issue when using string interpolation.

@kopax-polyconseil
Copy link
Author

kopax-polyconseil commented Apr 8, 2021

This fix the error :

text={t`dans l’année de tes 18 ans, obtiens l’aide financière pass Culture d’un montant de ${deposit.toString()} à dépenser dans l’application.`}

I am now concerned of the risk to have a bunch of error like this unspotted. deposit is of type string so that shouldn't be an issue.

@semoal
Copy link
Contributor

semoal commented Apr 8, 2021

I've tried to replicate the issue on the example/create-react-app application which uses create-reac-app under the hood and couldn't reproduce it.
I'm going to re-TRY to launch your application (if it's possible and will be more efficient) :/ Looks something related to react-native because in web (even with strictmode) doesn't log any error

@kopax-polyconseil
Copy link
Author

kopax-polyconseil commented Apr 8, 2021

I'm going to re-TRY to launch your application (if it's possible and will be more efficient)

I don't think you can, as explained in our README, if you want to run iOS, you will need our ios/GoogleService-Info.plist , and for android, there's also google-services.json and android/keystores/development.keystore.properties. I am unfortunately not allowed to share those privates.

How about bootstrapping a fresh react native application for running such test?

Looks something related to react-native because in web (even with strictmode) doesn't log any error

Considering the role of lingui, I have doubt about it, the JS engine should be working the same as in the browser. The one difference is what will be rendered in the DOM, lingui should return strings, strings should be in <Text></Text> (RN) or valid text node within a valid html tag (Web)

@semoal
Copy link
Contributor

semoal commented Apr 8, 2021

Yes, i'm going to bootstrap a react native application.

Give it a try if you can to reproduce it in a create-react-app example, update lingui to latest (make sure installed is 3.8.4) and tell me if you can reproduce it

@kopax-polyconseil
Copy link
Author

Yes, that's unfortunate, I am already on it.

@semoal
Copy link
Contributor

semoal commented Apr 8, 2021

Screenshot 2021-04-08 at 17 06 51
Can't reproduce it on a practically copy of setup of your application (metro-typescript-react-styled-components)

Any suggestion to force it?

@kopax-polyconseil
Copy link
Author

Thanks, I was still working on my boilerplate, so I am giving up. I will try to see if there's a way to start the application without those keys to see if we can investigate further.

@semoal
Copy link
Contributor

semoal commented Apr 8, 2021

Thanks, I was still working on my boilerplate, so I am giving up. I will try to see if there's a way to start the application without those keys to see if we can investigate further.

We could talk on a meeting if getting these keys are so risky for an external developer, I'm open to anything, just want to help :)

@kopax-polyconseil
Copy link
Author

Perhaps a peer programming session over meet can help solve this?

@semoal
Copy link
Contributor

semoal commented Apr 8, 2021

I'm sure that worst can't go =) today i'm busy at work, so if you want to try to reproduce and if not we speak tomorrow?

@kopax-polyconseil
Copy link
Author

kopax-polyconseil commented Apr 8, 2021

If you can't reproduce it I have doubt on how to reproduce it myself with a fresh native project.

I've checked with our team and starting the app without those secrets is not possible.

I am almost sure I can help you investigate if you want to dig to the root cause through a peer programming session, let me know if you want to schedule that tomorrow.

So far, I warned the dev team that v3 now bring issue with unicode chars and string literals. so far we have found alternatives for every dubious case we got, the worst alternative was to call a .toString() on a string.

@semoal
Copy link
Contributor

semoal commented Apr 8, 2021

Yes, let's schedule tomorrow. I have already picked 9:30 and 14:00, i'm free all the afternon

@kopax-polyconseil
Copy link
Author

OK, I've just scheduled on meet a 1h slot between 2pm and 3pm, you have the right to update the time, I am also free for the call the whole afternoon, looking forward with thanks !

@kopax-polyconseil
Copy link
Author

kopax-polyconseil commented Apr 8, 2021

We got another issue which is blocking for us. After releasing our application in app center with a real android build, it seems that the unicode chars are not replaced in every place, for exemple:

image

2nd thing is that the bug with the text on <SecondCard /> is not fixed (see pass-culture/pass-culture-app-native#849) and we do not have the issue anymore.

  1. Development environment can have issue with translation string interpolation
  2. Production environment still have the issue with unicode not beeing replaced

I think we may have to revert to v2 especially because of 2, we'll see tomorrow.

This is what we got in messages.po for this message:

#: src/features/profile/components/LoggedOutHeader.tsx:29
msgid "Tu as déjà un compte ?\\u00a0"
msgstr "Tu as déjà un compte ?\\u00a0"

@kopax-polyconseil
Copy link
Author

kopax-polyconseil commented Apr 9, 2021

Hi @semoal , I've just tested a production build with lingui 3.8.6 and I still have unicode issue in production, now messages.po looks like this:

msgid "Tu as déjà un compte ?/u00a0"
msgstr "Tu as déjà un compte ?/u00a0"

And view is still showing invalid chars:

image

We also have issue rendering our text at some place and this breaks our errors message, probably because the object returned is a function not a string:

image

@semoal
Copy link
Contributor

semoal commented Apr 9, 2021

Quick update:

One thing we can't fix without breaking others users, it's not stripping spaces at the end of strings.

Not inmediately fix

For example this case:

<Text>{t`EAN\u00a0`}</Text>

Should be like this if you want that space, or " ":

<Text>{t`EAN`}{\u00a0}</Text>

A suggestion is to add a configuration property to lingui like trimTranslations: true/false, but i've need some time this weekend to develop this technique.

Inmediately fix

Pending for your side confirmation

  • When you can tell me if we fixed the unicode chars in Production.

Pending for my side

  • I'm going to check asap if I can resolve the warning about can't use functions as child

@kopax-polyconseil
Copy link
Author

kopax-polyconseil commented Apr 9, 2021

Our v3 Fix

This allow us to not revert to v2 since we have no more visual regression:

https://github.com/pass-culture/pass-culture-app-native/pull/864/files

Unicode fix

This is how we solve the unicode issue, but now our messages.po does not interpolate the value, it just concatenates part of sentences from message file, ex:

https://github.com/pass-culture/pass-culture-app-native/pull/864/files#diff-cb154b350261abe067422a25938a2b4ff16788f9319324330616d7c75583d6a7L1218-R1219

Development variable interpolation broken

I'm going to check asap if I can resolve the warning about can't use functions as child

We still have the issue in development and I can't understand what can cause the behavior I showed you during the PP. It's not just a warning, the whole sentence doesn't get displayed and that's pretty bad while developing UI.

I hope this to be fixed, since this is not blocking our v3 migration, I can offer to do another peer programming session anytime if you need to have the environment.

Unicode trim fix

trimTranslations: true/false

I am much interested in that feature, can you also please confirm that unicode does not work only if they are around the sentence? Because we have:

t`Confirme ton e\u2011mail`
t`Je veux recevoir les actualités et les meilleures offres du pass Culture par e\u2011mail.`
{t`Autoriser l'envoi d'e\u2011mails`

And this seems to render correctly both in development and production

Unit tests

Jest did not exit one second after the test run has completed.

This usually means that there are asynchronous operations that weren't stopped in your tests. Consider running Jest with `--detectOpenHandles` to troubleshoot this issue.

Since the upgrade to v3, our unit test doesn't exit properly anymore. It hang and catch later the success signal, do you know why?

@kopax-polyconseil
Copy link
Author

Hello @semoal , I'd like to know if action will be taken regarding the issue we have with string, we still find a lot of view displayed like follow after migration to v3:

image

Also, our test execution time has increased from 4 to 8 min because of :

Jest did not exit one second after the test run has completed.

This usually means that there are asynchronous operations that weren't stopped in your tests. Consider running Jest with `--detectOpenHandles` to troubleshoot this issue.

This started after merging the v3 migration: pass-culture/pass-culture-app-native#845

Do you have a clue why the test are improperly configured with lingui v3 ?

@kopax-polyconseil
Copy link
Author

I've created two separate issues and suggest that we close that one so newcomers can get the proper feed and have this one linked.

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

No branches or pull requests

3 participants