Components: schematics(tree): Expected property shorthand in object literal

[Splaktar]

What is the expected behavior?

Running a schematic will not result in a lint failure.

What is the current behavior?

Running the tree schematic results in this failure:

ERROR: tree.component.ts:58:7 - Expected property shorthand in object literal ('{level}').

What are the steps to reproduce?

1. ng g @angular/material:tree tree

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

    "@angular/animations": "~8.0.0-rc.4",
    "@angular/cdk": "~8.0.0-rc.1",
    "@angular/common": "~8.0.0-rc.4",
    "@angular/compiler": "~8.0.0-rc.4",
    "@angular/core": "~8.0.0-rc.4",
    "@angular/forms": "~8.0.0-rc.4",
    "@angular/material": "^8.0.0-rc.1",
    "@angular/platform-browser": "~8.0.0-rc.4",
    "@angular/platform-browser-dynamic": "~8.0.0-rc.4",
    "@angular/router": "~8.0.0-rc.4",
    "hammerjs": "^2.0.8",

Plus [email protected]
OS: macOS Mojave

Is there anything else we should know?

The quick fix seems to change this:

    return {
      name: node.name,
      type: node.type,
      level: level,
      expandable: !!node.children
    };

to this:

    return {
      name: node.name,
      type: node.type,
      level,
      expandable: !!node.children
    };

This appears to be related to the object-literal-shorthand rule that is pulled in as part of "extends": "tslint:recommended",.

[devversion]

Can't it also be the other way around where people enforce that shorthand properties are not allowed?

[Splaktar]

@devversion certainly, but by default we should generate code that aligns with the default tslint rules in a new Angular CLI project. The "extends": "tslint:recommended" is new in Angular CLI version 7.3.0.

[devversion]

@Splaktar That is a fair point, but isn't the rule disabled by default in a new project? see:

https://github.com/angular/angular-cli/commit/2f262bb75f7b072237a5d14d8d10abe64aae2de1#diff-49a88607c73d1bbad7abaa45420d5882R67

[Splaktar]

It was, but that appears to have been removed at some point (along with a file rename). The latest version here does not include that rule.

[Splaktar]

Oh here it is, it was removed in https://github.com/angular/angular-cli/commit/a52fc53f0461c506c436b3a0450a2ba2984f8811.

[devversion]

:+1: in that case sounds reasonable to fix that. Personally I'm surprised that this rule is now enabled by default. The shorthand properties are not always desired (at least in my opinion :smile:)

[Splaktar]

I agree. I'm not a fan of this being the default, but that's what we have as of Angular CLI 7.3.0. @alan-agius4 @cexbrayat @kyliau was this intentional?

[alan-agius4]

@mgechev, was overseeing this.

That said, maybe we should fix lint by default when generating files? At the moment this is turned off by default and users need to use the —lint-fix so to address all or most cases?

Not sure if the component guys implement this as-well in their schematics. (Currently AFK)

[devversion]

@alan-agius4 AFAIK we can set up the schematic task for this on the Material/CDK side as well.. but not sure if that is a good pattern.

[alan-agius4]

@devversion, I am not referring to a global lint fix, but rather a lint fix on the generated files. This is currently implemented on all of the CLI schematics, but is off by default.

Ex: https://github.com/angular/angular-cli/blob/f651a2bd30fe812954dee6cc1fb2cc2e385a1bfd/packages/schematics/angular/pipe/index.ts#L115

[devversion]

@alan-agius4 I was exactly referring to that lint fix function which under-the-hood schedules the TslintFixTask.

See: https://github.com/angular/angular-cli/blob/c319cd054e4344737e274af41f46ef4d8a1c3aab/packages/schematics/angular/utility/lint-fix.ts#L45-L48

We could use that on our end as well but I'm not sure if that is a good pattern. Note that we already have a dependency on @schematics/angular so it wouldn't be a problem to even that function.

[alan-agius4]

Yeah I am not so sure of it either, but that will guarantee that generated code is following the user coding style. As really users are free to change the tslint rules as they see fit.

[devversion]

Yeah that would be ideal. Agreed. I'm tending to just revisit this after version 8 of Angular Material is released. It sounds like the rule that enforces the shorthand property is not set for _all_ CLI projects and it depends on the version. After version 8 we could have a patch release where we sort this out by running the TslintFix task (or whatever we think is a good pattern)

^{I could also imagine the CLI just running the fix task after running a generator schematic.. seems like a common case where you want to fix the updated/generated files}

[devversion]

Closing this in favor of https://github.com/angular/angular-cli/issues/14532. We should update that issue to be more generic. i.e. talk about lint failures caused by schematics. The proposal should be that lint fix runs automatically after any generate schematic. That would also simplify the --lint-fix flag for @schematics/angular generate schematics.

[angular-automatic-lock-bot[bot]]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{_This action has been performed automatically by a bot._}