mxmvshnvsk / i18n-unused

The static analyze tool for finding, marking and removing unused and missing i18n translations in your JavaScript project
MIT License
127 stars 21 forks source link

Fails with error with "mark" and "remove" #16

Closed CarlosNZ closed 2 years ago

CarlosNZ commented 2 years ago

It's working fine (and giving seemingly correct results) with "display-unused". However, when I run "mark-unused" or "remove-unused", I get an error like so:

 Successfully marked: /Users/carl/GitHub/opensupply/client/packages/common/src/intl/locales/ar/app.json
(node:18673) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'admin' of undefined
    at /Users/carl/GitHub/opensupply/client/node_modules/i18n-unused/dist/i18n-unused.cjs:469:51
    at /Users/carl/GitHub/opensupply/client/node_modules/i18n-unused/dist/i18n-unused.cjs:384:7
    at Array.reduce (<anonymous>)
    at applyToFlatKey (/Users/carl/GitHub/opensupply/client/node_modules/i18n-unused/dist/i18n-unused.cjs:382:16)
    at /Users/carl/GitHub/opensupply/client/node_modules/i18n-unused/dist/i18n-unused.cjs:468:37
    at Array.forEach (<anonymous>)
    at /Users/carl/GitHub/opensupply/client/node_modules/i18n-unused/dist/i18n-unused.cjs:468:22
    at Array.forEach (<anonymous>)
    at markUnusedTranslations (/Users/carl/GitHub/opensupply/client/node_modules/i18n-unused/dist/i18n-unused.cjs:465:35)
(Use `node --trace-warnings ...` to show where the warning was created)
(node:18673) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:18673) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

It deals with the first file (app.json) fine, but then gets no further.

Any ideas?

mxmvshnvsk commented 2 years ago

Hi. Can u attach part of app.json and your i18n-unused.config.js? it hard to say, where the problem by stack trace.

CarlosNZ commented 2 years ago

i18n-unused-test.zip

Thanks. This .zip contains:

You can clone our repo if you'd like to see more detail: https://github.com/openmsupply/openmsupply-client (Branch with this is called i18n-unused )

Many thanks. :)

mxmvshnvsk commented 2 years ago

Ok, I'll take a look this week.

mxmvshnvsk commented 2 years ago

I looked at your code. U have errors, because u use wrong deep translations. For example in distribution.json u use "label.group-by-item": "Group by Item", instead of object structure for t('label.group-by-item'). So, when my lib builds the structure t('label.group-by-item') back, it expands into an object, not a string, e.g.:

// wrong
{
  ...
  "label.group-by-item": "Group by Item",
  ...
}

// change to
{
  ...
  "label": {
    "group-by-item": "Group by Item"
  }
  ...
}

If I helped solve your problem, please, close an issue. Also will be glad for star =)

mxmvshnvsk commented 2 years ago

@CarlosNZ, do you have any updates?

CarlosNZ commented 2 years ago

Okay thanks, I'll take a closer look tomorrow and close the issue.

CarlosNZ commented 2 years ago

Hi, it seems like "flat" nested keys should be acceptable in i18n JSON files: https://www.codeandweb.com/babeledit/documentation/file-formats#flat-json

And indeed we are using t('label.group-by-item')-style using the https://www.i18next.com/ package throughout our app with no problems.

mxmvshnvsk commented 2 years ago

Ok, I see your point. I did a little research on this topic. I think it's possible to make implementation for flat json in next minor version at this week.

CarlosNZ commented 2 years ago

Excellent, thank you, I look forward to trying it out. :)

mxmvshnvsk commented 2 years ago

Hi. Thx for waiting =) U can try new 0.8.0 version. Now available option flatTranslations, boolean, switch locale builder to flat json mode.

CarlosNZ commented 2 years ago

Thanks @mxmvshnvsk ,

it's working better now (gets further), but it's still throwing an error at a certain point when running mark:

Successfully marked: /Users/carl/GitHub/openmsupply/client/packages/common/src/intl/locales/ar/app.json
 Successfully marked: /Users/carl/GitHub/openmsupply/client/packages/common/src/intl/locales/ar/common.json
 Successfully marked: /Users/carl/GitHub/openmsupply/client/packages/common/src/intl/locales/ar/replenishment.json
 Successfully marked: /Users/carl/GitHub/openmsupply/client/packages/common/src/intl/locales/en/app.json
(node:63505) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'default-sell-price' of undefined
    at applyToFlatKey.flatTranslations (/Users/carl/GitHub/openmsupply/client/node_modules/i18n-unused/dist/i18n-unused.cjs:485:51)
    at /Users/carl/GitHub/openmsupply/client/node_modules/i18n-unused/dist/i18n-unused.cjs:395:7
    at Array.reduce (<anonymous>)
    at applyToFlatKey (/Users/carl/GitHub/openmsupply/client/node_modules/i18n-unused/dist/i18n-unused.cjs:393:16)
    at /Users/carl/GitHub/openmsupply/client/node_modules/i18n-unused/dist/i18n-unused.cjs:484:37
    at Array.forEach (<anonymous>)
    at /Users/carl/GitHub/openmsupply/client/node_modules/i18n-unused/dist/i18n-unused.cjs:484:22
    at Array.forEach (<anonymous>)
    at markUnusedTranslations (/Users/carl/GitHub/openmsupply/client/node_modules/i18n-unused/dist/i18n-unused.cjs:481:35)

Oddly, there doesn't seem to be anything unique about "label.default-sell-price": "Default sell price", -- it's just one of several label. entries in that file.

mxmvshnvsk commented 2 years ago

@CarlosNZ, it's strange, because I cloned your repo and ran in i18n-unused branch:

Screenshot 2022-04-04 at 18 05 47

Try remove your node_modules or clear cache and install packages again.

CarlosNZ commented 2 years ago

Weird, I tried it on a different machine with a freshly-cloned repo and I'm still getting the same error.

Running on macOS 12.3.

I cloned this repo and ran the your tests and got similar errors:

Screen Shot 2022-04-05 at 9 34 35 AM
mxmvshnvsk commented 2 years ago

The current tests do not cover the codebase. I'll deal with this issue later. I'll try run clean installation on another machine later.

mxmvshnvsk commented 2 years ago

@CarlosNZ, I think I found the problem =) I looked at your update commit, you update package version, but not turn on flat json option in config, please, add follow option (it's in nested mode by default):

flatTranslations: true
CarlosNZ commented 2 years ago

@CarlosNZ, I think I found the problem =) I looked at your update commit, you update package version, but not turn on flat json option in config, please, add follow option (it's in nested mode by default):

Ah right, yes, my bad -- I didn't realise we needed to manually add that config option. Working well now, thank you :)

I'll close this issue now, but I just wanted to point out one tiny little thing you might want to address at some point: When running mark or remove your script removes a final line break in each file (even when there's nothing changed), which is different to how our auto-formatter (Prettier) does it (adds a final line break). So any file that we've manually saved shows up in the git diff after running this script, even if nothing has actually changed.

Screen Shot 2022-04-06 at 9 06 53 AM
mxmvshnvsk commented 2 years ago

Thx, I made note about line break.