Use ngDevMode to tree shake error messages in pipes

[dev054]

🚀 feature request

Description

Currently all the errors throwed by pipes are not being tree-shaked in production mode.

Relevant Package

@angular/common/(pipes)

Example:

angular/packages/common/src/pipes/case_conversion_pipes.ts

Lines 37 to 39 in 884f743

	if (typeof value !== 'string') {
	throw invalidPipeArgumentError(LowerCasePipe, value);
	}

Describe the solution you'd like

I have seen that you made a lot of progress with ngDevMode, so I think it would be a case of using it for the purpose of making the package smaller.

[alan-agius4]

I don’t think these errors should be removed when not ngDevMode is false.

Data can be non statically analysable such as from an API. In which case invalid value type should result in a meaningful and actionable errors.

Although it does feel weird that we allow undefined as an input just to throw that it’s an invalid value. Maybe this is for compatibility reasons?

[dev054]

I don’t think these errors should be removed when not ngDevMode is false.

@alan-agius4 Hmm, would you mind to share how do you decide an error message should be thrown in production mode or not? I'm asking it to understand as I saw ngDevMode being used in many places in @angular/router.

Although it does feel weird that we allow undefined as an input just to throw that it’s an invalid value. Maybe this is for compatibility reasons?

Maybe. I also found it strange (because even the async pipe which could be a problem, doesn't return undefined, but null), and I noticed that it was added in #37447.

[lazarljubenovic]

Data can be non statically analysable such as from an API. In which case invalid value type should result in a meaningful and actionable errors.

I thought Ivy now properly checks the type of pipes?

angular/packages/common/src/pipes/case_conversion_pipes.ts

Lines 32 to 35 in 884f743

	transform(value: string): string;
	transform(value: null|undefined): null;
	transform(value: string|null|undefined): string|null;
	transform(value: string|null|undefined): string|null {

The function will already scream at you during compilation for passing a wrong argument type.

[alan-agius4]

In this particular case undefined is a valid argument type, but when passed it causes a runtime exception.

[alan-agius4]

Actually never mind, I just noticed that the check in

angular/packages/common/src/pipes/case_conversion_pipes.ts

Line 36 in 884f743

if (value == null) return null;

is using double = and not triple.

So you are right 😊

[angular-robot]

Just a heads up that we kicked off a community voting process for your feature request. There are 20 days until the voting process ends.

Find more details about Angular's feature request process in our documentation.

[angular-robot]

Thank you for submitting your feature request! Looks like during the polling process it didn't collect a sufficient number of votes to move to the next stage.

We want to keep Angular rich and ergonomic and at the same time be mindful about its scope and learning journey. If you think your request could live outside Angular's scope, we'd encourage you to collaborate with the community on publishing it as an open source package.

You can find more details about the feature request process in our documentation.

[AndrewKushnir]

I think it should be safe to tree-shake away this error in prod mode (in all pipes within the case_conversion_pipes.ts script).

@ramthir this looks similar to other changes you made recently. Please let us know if you might be interested in helping with this code?

[ramthir]

I think it should be safe to tree-shake away this error in prod mode (in all pipes within the case_conversion_pipes.ts script).

@ramthir this looks similar to other changes you made recently. Please let us know if you might be interested in helping with this code?

Great @AndrewKushnir, I'll raise a PR on this. 👍

[angular-automatic-lock-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.}