i18n rule with check-id and check-text does not handle the ng-container tag
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 2.5k
- Forks
- 232
- PR merge metrics
- No merged PRs in 30d
Description
Reference: PR #506
Today I updated to version 4.2.1 of Codelyzer, as I was finding the linter was incorrectly reporting errors with our use of select ICU expressions (see: https://angular.io/guide/i18n#translate-select). PR #506 seems to have addressed those issues (thanks!). Unfortunately, I'm now finding that when I use ng-container tags with i18n ids (see: https://angular.io/guide/i18n#translate-text-without-creating-an-element) I get linting errors, that I don't expect.
I've cloned the repo, locally adding 2 tests to the suite. They're identical tests except for the use of different tags (ng-container vs span). As you'll see further below ng-container fails while span works fine:
66 it('NEW [ng-container]: should work with proper id', () => {
67 const source = `
68 @Component({
69 template: \`
70 <ng-container i18n="@@foo">Text</ng-container>
71 \`
72 })
73 class Bar {}
74 `;
75 assertSuccess('i18n', source, ['check-id', 'check-text']);
76 });
77
78 it('NEW [span]: should work with proper id', () => {
79 const source = `
80 @Component({
81 template: \`
82 <span i18n="@@foo">Text</span>
83 \`
84 })
85 class Bar {}
86 `;
87 assertSuccess('i18n', source, ['check-id', 'check-text']);
88 });
This is the output:
node@3bb72aa7d7e3:/codelyzer$ npm run test
npm info it worked if it ends with ok
npm info using npm@5.3.0
npm info using node@v8.5.0
npm info lifecycle codelyzer@4.2.1~pretest: codelyzer@4.2.1
npm info lifecycle codelyzer@4.2.1~test: codelyzer@4.2.1
> codelyzer@4.2.1 test /codelyzer
> rimraf dist && tsc && cp -r test/fixtures dist/test && mocha dist/test --recursive
i18n
check-id
✓ should work with proper id (50ms)
✓ should work with proper i18n attribute
✓ should work with proper id
✓ should work with proper id
1) NEW [ng-container]: should work with proper id
✓ NEW [span]: should work with proper id
✓ should fail with missing id string
✓ should fail with missing id
✓ should fail with missing id
8 passing (130ms)
1 failing
1) i18n check-id NEW [ng-container]: should work with proper id:
AssertionError: expected false to be true
at Function.assert.isTrue (node_modules/chai/lib/chai/interface/assert.js:332:31)
at Object.assertSuccess (dist/test/testHelper.js:161:17)
at Context.<anonymous> (dist/test/i18nRule.spec.js:30:26)
npm info lifecycle codelyzer@4.2.1~test: Failed to exec test script
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! codelyzer@4.2.1 test: `rimraf dist && tsc && cp -r test/fixtures dist/test && mocha dist/test --recursive`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the codelyzer@4.2.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! /home/node/.npm/_logs/2018-03-04T17_39_02_030Z-debug.log
One caveat, is that I don't see any tests in which both check-id and check-text are set so it's possible I shouldn't be configuring the linter to use both. Not sure. This is the line that I have in my tslint file:
"i18n": [true, "check-id", "check-text"]
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Run the repository's npm run test command and inspect the i18n check-id tests, including the failing ng-container case and the passing span case shown in dist/test/i18nRule.spec.js. Trace the rule behavior for check-id and check-text; done means the ng-container i18n="@@foo" test passes without regressing the existing cases.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- angular, typescript
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100