Bug: Keys Extractor does not extract key arrays

Question

Bug: Keys Extractor does not extract key arrays

seadonk opened this issue 3 years ago · 13 comments

Current behavior

A clear and concise description of what the bug is.
When using the selectTranslate override that accepts an array of keys, the keys manager will not recognize them.

Expected behavior

When using the selectTranslate override that accepts an array of keys, before adding the keys to the i18n files, the keys manager should recognize, by adding them with the extract tool, or showing them as missing with the find tool.

Steps To Reproduce

In any .ts file, use select translate with an array of keys, then run the find tool. Those keys will not be picked up.

a: string;
b: string;
c: string;
constructor(private translocoService: TranslocoService){
    this.translocoService.selectTranslate(['a','b','c']).subscribe(t => [this.a, this.b, this.c]=t);
}

The keys extractor/find tool will not recognize 'a','b','c' as missing keys.

Screenshots

If applicable, add screenshots to help explain your problem.

Keys manager config

Environment

- Keys manager version: XX 
- Node version: XX  
- Platform:

Debug logs

Additional context

I believe the keys extractor helpers simply need to be updated to search for an array parameter.

Contribution

I would like to make a pull request for this feature:
[x] Yes! 🚀
[ ] Maybe next time

Answer 1 · 2021-10-01T12:55:43.000Z

@seadonk will you be making a PR for it? 🙂

Answer 2 · 2021-10-01T15:26:49.000Z

@shaharkazaz Yes I plan on it, got delayed with a new computer. I'll have to figure out the process for contributing as this will be my first one.

Answer 3 · 2021-10-01T17:46:56.000Z

Feel free to approach of you need assistance 😃

Answer 4 · 2021-10-01T18:46:47.000Z

@shaharkazaz

Do I need additional setup besides npm install for running tests? I just get TypeError: string.replace is not a function at replacePathSepForRegex when I try to run any test in the project.
Is there a guide for contributing?
Do I need to run any of the scripts in package.json before the PR?

Answer 5 · 2021-10-01T22:23:43.000Z

There isn't any guide to contributing but I think that it's time to add one 😊

Basically all you need to run the build/tests is running npm i nothing else.

Contributing in a nut shell:

Make the changes
Add tests that cover the changes you made
Commit by running npm run commit
Open PR

Can you please post the call stack of the error? Do you get it after making changes?

Answer 6 · 2021-10-02T15:01:19.000Z

@seadonk did you manage? 🙂

Answer 7 · 2021-10-02T15:53:53.000Z

@shaharkazaz I did run npm i.
Two issues.

My environment does not know what PRODUCTION is, from the test script: test: "PRODUCTION=true jest"

$ npm run test

@ngneat/transloco-keys-manager@3.0.1 test D:\dev\github\transloco-keys-manager
PRODUCTION=true jest

PRODUCTION' is not recognized as an internal or external command,
operable program or batch file.
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! @ngneat/transloco-keys-manager@3.0.1 test: PRODUCTION=true jest
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the @ngneat/transloco-keys-manager@3.0.1 test script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR! C:\Users\bsn10\AppData\Roaming\npm-cache_logs\2021-10-02T15_49_21_193Z-debug.log

If I remove PRODUCTION=TRUE from the test script, I get this error:

$ npm run test

@ngneat/transloco-keys-manager@3.0.1 test D:\dev\github\transloco-keys-manager
jest

TypeError: string.replace is not a function
at replacePathSepForRegex (D:\Dev\github\transloco-keys-manager\node_modules\jest-regex-util\build\index.js:39:19)
at Array.map ()
at D:\Dev\github\transloco-keys-manager\node_modules@jest\core\node_modules\jest-config\build\normalize.js:999:59
at Array.reduce ()
at normalize (D:\Dev\github\transloco-keys-manager\node_modules@jest\core\node_modules\jest-config\build\normalize.js:784:14)
at readConfig (D:\Dev\github\transloco-keys-manager\node_modules@jest\core\node_modules\jest-config\build\index.js:230:74)
at async readConfigs (D:\Dev\github\transloco-keys-manager\node_modules@jest\core\node_modules\jest-config\build\index.js:414:26)
at async runCLI (D:\Dev\github\transloco-keys-manager\node_modules@jest\core\build\cli\index.js:220:59)
at async Object.run (D:\Dev\github\transloco-keys-manager\node_modules\jest-cli\build\cli\index.js:163:37)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! @ngneat/transloco-keys-manager@3.0.1 test: jest
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the @ngneat/transloco-keys-manager@3.0.1 test script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR! C:\Users\bsn10\AppData\Roaming\npm-cache_logs\2021-10-02T15_52_26_512Z-debug.log

Answer 8 · 2021-10-02T16:04:03.000Z

@seadonk I just did a fresh clone and it worked as expected, I'm running on OSX and my guess is that you are using Windows.
Let me make some adjustments and try again.

Answer 9 · 2021-10-02T16:27:17.000Z

@seadonk Pull master and LMK if the issue is resolved 🙂

Answer 10 · 2021-10-04T00:45:26.000Z

@shaharkazaz I am on windows.
I'm able to run the tests now, but the testRegex is not matching any tests.

$ npm test

@ngneat/transloco-keys-manager@3.0.1 test
cross-env PRODUCTION=true jest

jest-haste-map: Haste module naming collision: @ngneat/transloco-keys-manager
The following files share their name; please adjust your hasteImpl:
* \dist\package.json
* \package.json

No tests found, exiting with code 1
Run with --passWithNoTests to exit with code 0
In D:\Dev\github\transloco-keys-manager
331 files checked.
testMatch: - 0 matches
testPathIgnorePatterns: \node_modules\ - 331 matches
testRegex: tests\\\w+.spec.ts - 0 matches
Pattern: - 0 matches

If I remove the regex and allow jest to use the default regex: **/__tests__/**/*.[jt]s?(x), **/?(*.)+(spec|test).[tj]s?(x), it will incorrectly pick up all .ts files in the tests as tests. I've been having a lot of trouble with the Jest regexes in my other projects lately too.

Test Suites: 24 failed, 4 passed, 28 total
Tests: 29 passed, 29 total
Snapshots: 0 total
Time: 6.41 s

The only way I was able to get the tests to run was to edit the jest config, using a Glob and testMatch instead of testRegex.

Remove: testRegex: "__tests__\\/\\w+\\.spec\\.ts",
Add: testMatch: ["**/__tests__/**/*.spec.ts"],

PASS tests/deafultConfig.spec.ts
PASS tests/resolveProjectBasePath.spec.ts
PASS tests/resolveConfig.spec.ts
PASS tests/buildTranslationFiles.spec.ts

Test Suites: 4 passed, 4 total
Tests: 29 passed, 29 total
Snapshots: 0 total
Time: 4.162 s, estimated 6 s

Answer 11 · 2021-10-04T06:33:23.000Z

@seadonk I don't mind this change as long as all the specs are running 🙂
So are you managing now?

Answer 12 · 2021-10-04T15:20:09.000Z

@shaharkazaz I have the repo forked, changes made, tests written and passing.

When I run npm run commit, it is failing on Prettier, for lines that I did not modify. I'll see if I can get it running.

✖ prettier --write:
[error] tests\service\1.ts: SyntaxError: ',' expected. (93:7)
[error] 91 | translate('11');
[error] 92 | }
[error] > 93 | }),
[error] | ^
[error] 94 | autorun(() => {
[error] 95 | const fields = toJS(this.lightConnectStore.segmentationFields);
[error] 96 | segmentationFields.patchValue(fields);
tests\buildTranslationFiles.spec.ts 151ms

Answer 13 · 2021-10-07T06:40:15.000Z

@seadonk released in v3.0.2, nice work on the PR, hope to see you contributing again 🙂