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

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

Closed
CarlosNZ opened this issue Mar 17, 2022 · 17 comments
Closed

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

CarlosNZ opened this issue Mar 17, 2022 · 17 comments

Comments

@CarlosNZ
Copy link

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
Copy link
Owner

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
Copy link
Author

CarlosNZ commented Mar 17, 2022

i18n-unused-test.zip

Thanks. This .zip contains:

  • i18n-unused.config.js
  • app.json (which seems to be the only file that is behaving correctly)
  • common.json (which I think is the next file analysed which throws the error)

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
Copy link
Owner

Ok, I'll take a look this week.

@mxmvshnvsk
Copy link
Owner

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
Copy link
Owner

@CarlosNZ, do you have any updates?

@CarlosNZ
Copy link
Author

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

@CarlosNZ
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

CarlosNZ commented Mar 28, 2022

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

@mxmvshnvsk
Copy link
Owner

mxmvshnvsk commented Apr 3, 2022

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
Copy link
Author

CarlosNZ commented Apr 3, 2022

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
Copy link
Owner

@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
Copy link
Author

CarlosNZ commented Apr 4, 2022

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
Copy link
Owner

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
Copy link
Owner

@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
Copy link
Author

CarlosNZ commented Apr 5, 2022

@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
Copy link
Owner

Thx, I made note about line break.

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

2 participants