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

babel-plugin-lingui-extract-messages doesn't work when targeting IE 11 #32

Closed
akkie opened this issue Jul 6, 2017 · 22 comments
Closed

Comments

@akkie
Copy link

akkie commented Jul 6, 2017

If I add the browser ie 11 to my babel-preset-env config, then I get some errors if I try to build my application:

[info] Module build failed: TypeError: ...: Cannot read property 'reduce' of undefined
[info]     at collectMessage (...\node_modules\babel-plugin-lingui-extract-messages\lib\index.js:24:27)

I'm not sure if this has to do with js-lingui or if it's an issue from another component.

@tricoder42
Copy link
Contributor

Thank you for bug report!

Unfortunately, I'm not able to replicate it. There's major release coming in next few days, could you please try it?

npm install --save lingui-react@next
npm install --save-dev babel-preset-lingui-react@next
npm install --global lingui-cli@next

And then run: lingui extract

babel-plugin-lingui-extract-messages is no longer part of babel-preset-lingui-react. It's add manually by CLI when running lingui extract (the reason is performance - messages shouldn't be extracted on every babel build).

Moreover, babel-plugin-lingui-extract-messages warns about edge cases which would case the bug you've just described.

@akkie
Copy link
Author

akkie commented Jul 7, 2017

OK. Will test it if the new version is released.

the reason is performance - messages shouldn't be extracted on every babel build

Could it not test if the catalog is up-to-date based on a timestamp cached for every catalog. So that the catalog gets automatically built if something has changed?

@tricoder42
Copy link
Contributor

That could work, but still you need a catalog only when extracting messages for translators, not for development. Also, having babel plugin added in lingui extract has another advantage, that it works also for projects which can't edit the babel config.

@tricoder42
Copy link
Contributor

@akkie I've just published new major version, could you please try if the bug is still there.

There're few backward incompatible changes covered in Migration guide. Most of changes are low-level api, only few affects react

@tricoder42
Copy link
Contributor

I've just tried following .babelrc config:

{
  "presets": [
    [
        "env", 
        {
            "targets": {
                "browsers": ["ie 11"]
            }
        }
    ],
    "react",
    "lingui-react"
  ]
}

in tutorial repository, but I still get no error.

Closing now, feel free to reopen when you have more info.

@akkie
Copy link
Author

akkie commented Jul 10, 2017

I try to migrate to 1.0.1, but for me it's not really clear how a catalog should look like. Previously we had message files in the form:

{
  "key": "value"
}

But now we have a catalog in the form:

type Catalog = {
  messages: Messages,
  languageData: {
    // required in production
    plurals: Function
  }
}

Does this mean that the files are now structured like this:

{
  "messages": {
    "key": "value"
  }
}

If yes, then this part of the documentation is outdated.

@tricoder42
Copy link
Contributor

Yes, good catch, sorry for mistake.

It should be:

<I18nProvider language="en" catalogs={{ en: { messages } }} development={dev}>

(just wrap messages in dictionary)

However, it's better to load compiled message catalogs during development, so it works exactly as in production:

import catalog from 'locale/en/messages.js'

const App = () => (
  <I18nProvider language="en" catalogs={{ en: catalog }} development={dev}>
    ...
  </I18nProvider>
)

You can compile catalog with lingui compile. (I guess we could add webpack loader to handle compilation automatically.)

@tricoder42
Copy link
Contributor

Tutorial fixed in #36

@akkie
Copy link
Author

akkie commented Jul 10, 2017

It's a bit confusing sometimes you use:

import catalog from 'locale/en/messages.js'

and sometimes:

import messages from 'locale/cs/messages.json'

@tricoder42
Copy link
Contributor

I went through docs and it's consistent now.

Recommended way is import catalog from 'locale/en/messages.js.

The other way is supported for backward compatibility and when you can't/don't want to compile messages. I should probably remove it from docs completely, so it's less confusing.

@akkie
Copy link
Author

akkie commented Jul 10, 2017

Yes, I think it's better to remove it or to explain the difference between the two file formats.

JSON: Source file from which the catalog will be built
JS: Compiled message catalog

Anyway, I cannot get the lib to work. Is it normal that the compiled format has the form:

{
  l: {},
  m: {}
}

?

@tricoder42
Copy link
Contributor

My apologies! I was fixing every little detail before the major release and I forgot to document the most important part: unpackCatalog from lingui-i18n

The message catalog is "minified" and need to be unpacked before loaded:

import { unpackCatalog } from 'lingui-i18n'
import catalog from 'locale/cs/messages.js'

const App = () => (
  <I18nProvider language="cs" catalogs={{ cs: unpackCatalog(catalog) }} development={dev}>
    <Inbox />
  </I18nProvider>
)

I've updated tutorial in #37

Also, take a look at example how to load messages with webpack. It looks way cleaner.

@akkie
Copy link
Author

akkie commented Jul 10, 2017

Thanks 👍

Also, take a look at example how to load messages with webpack. It looks way cleaner.

Yea, I use this in a similar manner.

If I compile my German messages file, then the compiled catalog contains only the English variant.

My source file contains as example:

{
  "This field is required": "Dies ist ein Pflichtfeld",
}

The compiled catalog contains then:

{
  "This field is required": "This field is required",
}

It seems that it uses the key also as value.

@tricoder42
Copy link
Contributor

I see. I was able to replicate this error by using old-style message catalog. Format of message catalog have changed as stated in migration guide (https://lingui.gitbooks.io/js/content/guides/migration-1.html#cli).

All you need is run lingui extract. It will merge translations from previous catalog (even if it's in old format) and generate a new one.

Finally, lingui compile should work as expected.

After running lingui extract, the message catalog should look like this:

{
  "msg.heading": {
    "translation": "Ukázková aplikace",
    "defaults": "Application Example",
    "origin": [
      [
        "test/fixtures/App.js",
        5
      ]
    ]
  },
  "Example of <0>react-trans</0> usage.": {
    "translation": "Příklad použití <0>react-trans</0>.",
    "origin": [
      [
        "test/fixtures/App.js",
        7
      ]
    ]
  },
  ...
}

@akkie
Copy link
Author

akkie commented Jul 10, 2017

Sorry, I should better double check the documentation 😉

Can it be the case that lingui extract doesn't work with flow?

SyntaxError: .../src/apis/I18nAPI.js: Unexpected token, expected , (15:29)
  13 |    * @returns The message catalog for the given language.
  14 |    */
> 15 |   async fetchCatalog(language: string): Promise<Object> {
     |                              ^
  16 |     const catalog = await import(
  17 |       /* webpackMode: "lazy", webpackChunkName: "i18n-[index]" */
  18 |       `locale/${language}/messages.js`);

@tricoder42
Copy link
Contributor

tricoder42 commented Jul 10, 2017

I'm using flow as well without any issues. lingui-cli only extends your .babelrc config, but I guess you use react plugin, which adds support for flow syntax.

Is fetchCatalog class method? Standalone functions must have function keyword after async:

async function fetchCatalog (language: string) {
   ...
}

without function, I'm getting the same error:

SyntaxError: /Users/tricoder/Projects/lingui/lingui-web/src/lingui/locale/ui/I18nLoader/I18nLoader.js: Unexpected token, expected => (9:13)
   7 | import { selector } from 'lingui/locale'
   8 |
>  9 | async fetchCatalog (language: string) {

EDIT: Actually, it's different error...

@akkie
Copy link
Author

akkie commented Jul 10, 2017

Is fetchCatalog class method? Standalone functions must have function keyword after async:

Yes, it's a class method.

but I guess you use react plugin, which adds support for flow syntax.

I use neutrino.js which does all the magic. Neutrino pulls in dynamically babel-preset-react. This means the preset isn't loaded in the .babelrc file.

Any idea for a workaround?

@tricoder42
Copy link
Contributor

Isolate I18nApi component, so it doesn't have any translations in it and then ignore it using srcPathIgnorePatterns config:

{
  "lingui": {
    "srcPathIgnorePatterns": [
      "/node_modules/",
      "I18nApi.js$"
    ]
}

Please update to latest lingui-cli@1.0.3. I fixed pattern matching so it excludes also files, not only directories.

Does it work for you as a temporary workaround? I don't have any translations inside I18nLoader component, it only takes care of loading translations.

@akkie
Copy link
Author

akkie commented Jul 10, 2017

I use flow also in react components, so your proposal won't work. I've created a i18n env in babel which adds the necessary presets and plugins.

"env": {
  "i18n": {
    "plugins": [
      "transform-class-properties",
      "transform-es2015-classes",
      "transform-object-rest-spread",
      "syntax-dynamic-import"
    ],
    "presets": ["react"]
  }
}

Then I can execute cross-env BABEL_ENV=i18n lingui extract.

I'll ask in the Neutrino.js Slack Channel if there is a way to execute third party commands with the generated babel configuration.

@akkie
Copy link
Author

akkie commented Jul 10, 2017

@tricoder42 Is it intended that the origin entry in the messages.json points multiple times to the same line in the same file?

An example:

{
  "Dashboard": {
    "translation": "Dashboard",
    "origin": [
      [
        "src\\components\\MainMenu\\MainMenu.jsx",
        86
      ],
      [
        "src\\components\\MainMenu\\MainMenu.jsx",
        86
      ],
      [
        "src\\components\\MainMenu\\MainMenu.jsx",
        86
      ],
      [
        "src\\components\\MainMenu\\MainMenu.jsx",
        86
      ],
      [
        "src\\components\\MainMenu\\MainMenu.jsx",
        86
      ]
    ]
  },
...
}

@tricoder42
Copy link
Contributor

No, that's definitely bug.

@tricoder42
Copy link
Contributor

Fixed in #39, lingui-cli@1.0.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants