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.

Sign up for GitHub

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

Jump to bottom

Pipe generated transform method has wrong parameters #12602

Closed
tchatel opened this issue Oct 15, 2018 · 3 comments · Fixed by #14785
Closed

Pipe generated transform method has wrong parameters #12602

tchatel opened this issue Oct 15, 2018 · 3 comments · Fixed by #14785
Labels
area: schematics/angular freq1: low Only reported by a handful of users who observe it rarely severity1: confusing
Milestone
needsTriage

Comments

@tchatel
Copy link

tchatel commented Oct 15, 2018

Bug Report or Feature Request (mark with an x)

- [x] bug report -> please search issues before submitting
- [ ] feature request

Command (mark with an x)

- [ ] new
- [ ] build
- [ ] serve
- [ ] test
- [ ] e2e
- [x] generate
- [ ] add
- [ ] update
- [ ] lint
- [ ] xi18n
- [ ] run
- [ ] config
- [ ] help
- [ ] version
- [ ] doc

Versions

All versions are affected, including v7.0.0-rc.3.

Repro steps

ng generate pipe ...

Problem

The generated method transform has wrong parameters. The second parameter, args, will not receive all the pipe arguments, but only the first one, and other arguments are lost.

The generated method is:

transform(value: any, args?: any): any {
  return null;
}

A right version should be:

transform(value: any, ...args: any[]): any {
  return null;
}
@alan-agius4
Copy link
Collaborator

Not sure about this, because I think in general it preferred to specify all the parameter needed.

Example;

 transform(value: any, argOne: number, argTwo?: number): any {

@ngbot ngbot bot added this to the needsTriage milestone Oct 16, 2018
@alan-agius4 alan-agius4 added freq1: low Only reported by a handful of users who observe it rarely and removed area: schematics/angular labels Oct 16, 2018
@ngbot ngbot bot removed this from the needsTriage milestone Oct 16, 2018
@ngbot ngbot bot added this to the needsTriage milestone Oct 16, 2018
@tchatel
Copy link
Author

tchatel commented Oct 16, 2018

Yes, I agree, you should always rewrite the method with meaningful parameters and right types, even the first one (value).
It's what I explain when I give Angular trainings. But I don't like having to tell that the generated code is wrong. Generated code should not be wrong, it should not mean that all the pipe arguments will be sent in the second method parameter, since it's not what happen.

Perhaps it may generate something like that:

transform(value: any, arg1: any, arg2: any): any {

or any other syntax that won't confuse the users.

LakhyariMs added a commit to LakhyariMs/angular-cli that referenced this issue Jun 15, 2019
@LakhyariMs
fix(@schematics/angular): improve pipe signature
389ea92 
Currently , the CLI generates :

```typescript

transform(value: any , args?: any)

```

With this commit , it generate :

```typescript

transform(value: any, ...args?: any[])

```

Which conforms to the official doc

Fixes angular#12602
@LakhyariMs LakhyariMs mentioned this issue Jun 15, 2019
Merged
mgechev pushed a commit that referenced this issue Jun 20, 2019
@LakhyariMs @mgechev
fix(@schematics/angular): improve pipe signature
5339611 
Currently , the CLI generates :

```typescript

transform(value: any , args?: any)

```

With this commit , it generate :

```typescript

transform(value: any, ...args?: any[])

```

Which conforms to the official doc

Fixes #12602
mgechev pushed a commit that referenced this issue Jun 20, 2019
@LakhyariMs @mgechev
fix(@schematics/angular): improve pipe signature
1ca19fb 
Currently , the CLI generates :

```typescript

transform(value: any , args?: any)

```

With this commit , it generate :

```typescript

transform(value: any, ...args?: any[])

```

Which conforms to the official doc

Fixes #12602
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
area: schematics/angular freq1: low Only reported by a handful of users who observe it rarely severity1: confusing
Projects
None yet
Milestone
needsTriage
Development

Successfully merging a pull request may close this issue.

fix(@schematics/angular): improve pipe signature LakhyariMs/angular-cli
2 participants
@tchatel @alan-agius4