Setting the highlighted code to empty string will not reflect the value change

[FrancescoBorzi]

Bug Report or Feature Request (mark with an x)

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

OS and Version?

Ubuntu 18.04

Versions

Angular CLI: 9.0.7
Node: 12.14.0
OS: linux x64

Angular: 9.0.6
... animations, common, compiler, compiler-cli, core, forms
... platform-browser, platform-browser-dynamic, router
Ivy Workspace: Yes

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.900.3
@angular-devkit/build-angular     0.900.7
@angular-devkit/build-optimizer   0.900.7
@angular-devkit/build-webpack     0.900.7
@angular-devkit/core              9.0.3
@angular-devkit/schematics        9.0.7
@angular/cli                      9.0.7
@ngtools/webpack                  9.0.7
@schematics/angular               9.0.7
@schematics/update                0.900.7
rxjs                              6.5.4
typescript                        3.7.5
webpack                           4.41.2

Description

I'm having a hard time updating from version 3.x to version 4.x as whenever in my app I change the code from a non-empty string to an empty string (''), the change will not be reflected

Repro steps

I've created a minimal application that reproduces the issue:

1. Clone this app https://github.com/FrancescoBorzi/ngx-highlightjs-bug-reproduction
2. npm i && ng serve
3. Open the app and delete the content of the input. You'll see when the input is empty, the highlight is not being updated

<input [(ngModel)]="code">

<pre>
  <code [highlight]="code"></code>
</pre>

I tested on both versions v4.0.2 and v4.1.0-beta, the bug is the same.

It worked correctly on version v3.0.3

Desired functionality

Should correctly update the view displaying an empty string

Mention any other details that might be useful

This bug appeared in 4.x, it worked fine in version 3.x

[MurhafSousli]

Please use stackblitz for reproduction https://stackblitz.com/edit/ngx-highlightjs

If you look at the source code

ngx-highlightjs/projects/ngx-highlightjs/src/lib/highlight.ts

Lines 52 to 60 in 76287be

	ngOnChanges(changes: SimpleChanges) {
	if (
	changes.code &&
	changes.code.currentValue &&
	changes.code.currentValue !== changes.code.previousValue
	) {
	this.highlightElement(this.code, this.languages);
	}
	}

As long as you don't pass null value, it should set the text. can you confirm that you input is not passing a null value

[FrancescoBorzi]

@MurhafSousli here you go: https://stackblitz.com/edit/ngx-highlightjs-sftnnx

try to delete the text from the input and you'll see the bug

I think removing this line changes.code.currentValue && (line 55) will fix the issue

[FrancescoBorzi]

@MurhafSousli any news on this? were you able to reproduce the issue?

[MurhafSousli]

Here https://stackblitz.com/edit/ngx-highlightjs-yp5rvq?file=src/app/home/home.component.ts

It turns out that if you want bind code to an input or textarea, you need to turn of the line numbers feature

[FrancescoBorzi]

@MurhafSousli why closing this issue? that link of yours still has the same bug:

[FrancescoBorzi]

@MurhafSousli

[Helias]

Confirmed.

[MurhafSousli]

The thing the value you are passing is not EMPTY, it is null value
see the console here

https://stackblitz.com/edit/ngx-highlightjs-yp5rvq?file=src%2Fapp%2Fhome%2Fhome.component.ts

When you pass null, replace with space

[FrancescoBorzi]

@MurhafSousli that way you are hacking it to be a space ' ' that is different than empty string ''.

The bug is that '' is falsy in js, so the condition if (value) will be false when value is ''

[MurhafSousli]

Thats right, maybe we can check if the value !== undefined instead

[FrancescoBorzi]

@MurhafSousli yes, or even better just set to '' (empty string) whenever the value is '' (empty string) or null or undefined

[MurhafSousli]

Fixed in v4.1.1, working stackblitz https://stackblitz.com/edit/ngx-highlightjs-bz2bqz

[estanglerbm]

I think this fix was undone by commit d60dc10 (for issue #142), such as in 4.1.3 (for Angular 11).

I realize that you may want to have <code highlight=""> do nothing (for one use case). But need some way to clear the pieces that ngx-highlightjs created before the value went blank (in the other use case).

[MurhafSousli]

@estanglerbm can you provide a stackblitz to demonstrate the use case?

[estanglerbm]

https://stackblitz.com/edit/angular-ivy-ypchsm?file=src/app/app.component.html

If you click the yellow box for JSON or SQL, and then click on Blank, then the area under "Issue #119" doesn't get updated.

If you click any yellow box except SQL, then click SQL, then any other yellow box, then the area under "Issue Other" doesn't get updated ever again (except perhaps for SQL).

The "Issue #119" area is about just highlighting code, and handling a blank value.
The "Issue Other" area is about whether or not to highlight the code (and if not, then showing the unhighlighted code).

This affects 4.1.3 with Angular 11, too.

[cbejensen]

I am experiencing this issue, but it's easily solvable:

<code [highlight]="value || ' '"></code>

That being said, I think the content should be updated even if it's null or undefined.

[estanglerbm]

I know (my stackblitz illustrated the space workaround, too), but there may be cases where a space character is not desired.

But the real issue here, as I previously mentioned, is that there are two use cases: one where you want nothing to happen, and one where you want to clear the area. There should be a mechanism to do both things (and a highlight="" would probably do one of them, and another mechanism does the other, or maybe solve the situation differently).

Otherwise, you're just going to continue to have future commits conflicting on this, because some people expect highlight="" to do nothing, and others expect highlight="" to clear the area.

[MurhafSousli]

@estanglerbm I would recommend listing all the use cases and their expected behaviour as bullet points to easily follow on them. I find it confusing to read the use cases and how it should behave from the comment or from the stackblitz.

[FrancescoBorzi]

@MurhafSousli I'd recommend a test case which will prevent this issue from being introduced again by mistake

[MurhafSousli]

Ok, it is fixed in 6.1.1 and I added test cases for that.

Now the expected behavior is the following:

• If you pass a null value, nothing is gonna change, it will not highlight, and it will not change a previously highlighted code.
• If you pass an empty string, it will clear the highlighted content.

[estanglerbm]

@MurhafSousli Fantastic. (I can't test it for a while, but the behavior sounds good.) Thanks.