fix: regressions in 4.4.0 #671
Conversation
package.json
Outdated
| @@ -86,7 +86,7 @@ | |||
| "rxjs-compat": "^6.1.0", | |||
| "ts-node": "^6.0.2", | |||
| "tslint": "^5.10.0", | |||
There was a problem hiding this comment.
Would not it be interesting to downgrade tslint to "~5.9.1" like angular CLI ?
src/selectorNameBase.ts
Outdated
| @@ -148,7 +148,7 @@ export class SelectorValidatorWalker extends Lint.RuleWalker { | |||
| } | |||
|
|
|||
| private validateProperty(p: ts.PropertyAssignment): boolean { | |||
| return ts.isStringLiteralLike(p.initializer) && ts.isIdentifier(p.name) && p.name.text === 'selector'; | |||
| return p.initializer.kind === ts.SyntaxKind.StringLiteral && ts.isIdentifier(p.name) && p.name.text === 'selector'; | |||
There was a problem hiding this comment.
ts.isStringLiteral(p.initializer)?
src/util/utils.ts
Outdated
| export const isSimpleTemplateString = (e: any): e is ts.SyntaxKind.FirstTemplateToken | ts.StringLiteralLike => { | ||
| return ts.isStringLiteralLike(e) || e.kind === ts.SyntaxKind.FirstTemplateToken; | ||
| export const isSimpleTemplateString = (e: any): e is ts.SyntaxKind.FirstTemplateToken | ts.StringLiteral => { | ||
| return e.kind === ts.SyntaxKind.StringLiteral || e.kind === ts.SyntaxKind.FirstTemplateToken; |
There was a problem hiding this comment.
ts.isStringLiteral(e)?
src/preferInlineDecoratorRule.ts
Outdated
| @@ -102,3 +103,7 @@ export class PreferInlineDecoratorWalker extends NgWalker { | |||
| this.addFailureAt(decoratorStartPos, property.getWidth(), getFailureMessage(), fix); | |||
| } | |||
| } | |||
|
|
|||
| function isSameLine(sourceFile: ts.SourceFile, pos1: number, pos2: number) { | |||
There was a problem hiding this comment.
Any reason to not use tsutils?
There was a problem hiding this comment.
tsutils isn't a dependencies/peerDependencies of Codelyzer
There was a problem hiding this comment.
You're right, however it's a tslint dependency. Anyway, can't we add it to codelyzer dependencies?
There was a problem hiding this comment.
IMO, sometimes, it's safer to duplicate some code lines than to be linked to a third-party library.
There was a problem hiding this comment.
In this case, it would be better to move this function to util.
src/index.ts
Outdated
| @@ -25,6 +25,7 @@ export { Rule as NoUnusedCssRule } from './noUnusedCssRule'; | |||
| export { Rule as PipeImpureRule } from './pipeImpureRule'; | |||
| export { Rule as PipeNamingRule } from './pipeNamingRule'; | |||
| export { Rule as PreferOutputReadonlyRule } from './preferOutputReadonlyRule'; | |||
| export { Rule as PreferInlineDecorator } from './preferInlineDecoratorRule'; | |||
There was a problem hiding this comment.
nit: I guess these exports are alphabetically ordered, so it would come before preferOutputReadonly.
|
LGTM. |
@angular/corepreferInlineDecoratorRuleWe should introduce a public API guard to make sure we don't hit
regressions like the second one.
Fix #669 #670