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.

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

when using point.pattern: null data points lead to 'Error: <use> attribute y: Expected length, "NaN".' in ESM module #2107

Closed
jonka-blip opened this issue May 25, 2021 · 11 comments
Labels

Comments

@jonka-blip
Copy link

jonka-blip commented May 25, 2021

Description

When lines have null data points (as in "line.connectNull"), the js code throws the Error: attribute y: Expected length, "NaN". This happens if the code gets imported via 'import { bb, line } from "billboard.js/dist/billboard.esm.js"'

a quick fix is to change Line 249 in point.ts to

$$.getYScaleById(id, isSub)($$.getBaseValue(d) || 0);

but I am not sure if this is the appropriate fix

@netil
Copy link
Member

netil commented May 25, 2021

Hi @jonka-blip, could you provide reproducible code?

@jonka-blip
Copy link
Author

@jonka-blip
Copy link
Author

The issue gets triggered if a point:pattern for the circle is used. You can see the Error in js console. I am using latest Chrome.
thx

@watnab
Copy link

watnab commented May 27, 2021

When lines have null data points
point:pattern

var chart = bb.generate({bindto: "#lineChart",
	data: {type: "line",
		columns: [ ["custom_id",1,null] ],
	},
	"point": {
		"pattern": ["<circle cx=\"4\" cy=\"4\" r=\"4\" />"]
	}
});

@watnab
Copy link

watnab commented May 27, 2021

.attr("y", yPosFn2)

I couldn't see what value yPosFn2 must return when data point is null.

@jonka-blip jonka-blip changed the title null data point leads to 'Error: <use> attribute y: Expected length, "NaN".' in ESM module when using point.pattern: null data points lead to 'Error: <use> attribute y: Expected length, "NaN".' in ESM module May 27, 2021
@jonka-blip
Copy link
Author

@watnab and why gets the error triggered only if point.pattern is defined? 🤔

@watnab
Copy link

watnab commented May 27, 2021

why gets the error triggered only if point.pattern is defined

custom may mean "pattern is defined" is defined.

const yPosFn2 = d => yPosFn(d) - height / 2;

  • yPosFn2 is defined with yPosFn in custom .
  • yPosFn is called even if pattern is not defined.
  • It seems yPosFn will return undefined when data value is null.
  • yPosFn(d) - height / 2 will be NaN when yPosFn returns undefined.

what value yPosFn2 must return when data point is null.

It may be good yPosFn2 returns undefined when yPosFn returns undefined.

@watnab
Copy link

watnab commented May 27, 2021

It may be good yPosFn2 returns undefined when yPosFn returns undefined.

undefined will cause Error: attribute y: Expected length, "undefined".

  • NaN -> NG
  • undefined -> NG
  • null -> NG
  • 0 -> OK?

@watnab
Copy link

watnab commented May 27, 2021

My test was wrong. undefined won't cause Error.
"d3.v6.min.js" has code like null == e ? this.removeAttribute(t) : this.setAttribute(t, e).

why gets the error triggered only if point.pattern is defined

The reason may be that null == NaN will return false.

@netil
Copy link
Member

netil commented May 28, 2021

Confirmed the issue. It happens to determine custom data points position for nullish data.
I'll do the fix.

BTW, there's no need to use custom data point for <circle cx=\"4\" cy=\"4\" r=\"4\" />.
You can control circle's radius using point.r option.

netil added a commit to netil/billboard.js that referenced this issue May 28, 2021
Exclude position computation for nullish data

Ref naver#2107
netil added a commit to netil/billboard.js that referenced this issue May 28, 2021
Exclude position computation for nullish data

Ref naver#2107
@netil netil closed this as completed in 8c198f2 May 28, 2021
@jonka-blip
Copy link
Author

cool 😎 ❤️

github-actions bot pushed a commit that referenced this issue Jun 25, 2021
# [3.1.0](3.0.3...3.1.0) (2021-06-25)

### Bug Fixes

* **axis:** Fix axis.x.padding value setting ([5b4b509](5b4b509)), closes [#2038](#2038)
* **axis:** fix handling x padding value ([489d47a](489d47a)), closes [#2038](#2038)
* **candlestick:** fix to set expand state ([a055b20](a055b20)), closes [#2036](#2036)
* **Chart:** Handle nullish properties from API extendings safely ([6cbf64a](6cbf64a)), closes [#2132](#2132) [#2134](#2134)
* **data:** Fix duplicated data.onclick call  ([b4c5dc2](b4c5dc2)), closes [#2104](#2104)
* **data:** Fix nullish data filtering for grouped data ([af19370](af19370)), closes [#2096](#2096)
* **gauge:** Fix incorrect rendering when gauge.min is given ([31fc981](31fc981)), closes [#2123](#2123)
* **point:** Fix custom point for nullish data ([8c198f2](8c198f2)), closes [#2107](#2107)
* **region:** fix region append position ([2b50443](2b50443)), closes [#2067](#2067)
* **size:** enhance applying height value ([0664a60](0664a60)), closes [#2086](#2086)
* **tooltip:** Correct the type of selectedData ([05b694d](05b694d)), closes [#2056](#2056) [#2058](#2058)
* **types:** fix missing candlestick export ([f218939](f218939)), closes [#2007](#2007)
* **types:** updated bar/candlestick options types ([d89c3f3](d89c3f3)), closes [#2043](#2043)
* **zoom:** Fix incorrect tooltip position ([689bfdf](689bfdf)), closes [#2095](#2095)
* **zoom,grid:** fix grid line pos during zoom ([e84a4f1](e84a4f1)), closes [#2156](#2156)

### Features

* **all:** contain inline css prop setting ([fde6a89](fde6a89)), closes [#2076](#2076)
* **api:** Intent to ship append load ([8076795](8076795)), closes [#2140](#2140)
* **data:** Intent to ship data.onshown/onhidden ([af98eb7](af98eb7)), closes [#2146](#2146)
* **data.labels:** Intent to ship data.labels.backgroundColors ([e0b2fed](e0b2fed)), closes [#1954](#1954)
* **subchart:** Intent to ship subchart.init.range option ([967bf1b](967bf1b)), closes [#2037](#2037)
* **subchart:** Intent to ship subchart.showHandle ([219bff3](219bff3)), closes [#2044](#2044)
netil pushed a commit that referenced this issue Jun 25, 2021
* **Chart:** Handle nullish properties from API extendings safely ([6cbf64a](6cbf64a)), closes [#2132](#2132) [#2134](#2134)
* **data:** Fix duplicated data.onclick call  ([b4c5dc2](b4c5dc2)), closes [#2104](#2104)
* **data:** Fix nullish data filtering for grouped data ([af19370](af19370)), closes [#2096](#2096)
* **gauge:** Fix incorrect rendering when gauge.min is given ([31fc981](31fc981)), closes [#2123](#2123)
* **point:** Fix custom point for nullish data ([8c198f2](8c198f2)), closes [#2107](#2107)
* **region:** fix region append position ([2b50443](2b50443)), closes [#2067](#2067)
* **size:** enhance applying height value ([0664a60](0664a60)), closes [#2086](#2086)
* **tooltip:** Correct the type of selectedData ([05b694d](05b694d)), closes [#2056](#2056) [#2058](#2058)
* **zoom:** Fix incorrect tooltip position ([689bfdf](689bfdf)), closes [#2095](#2095)
* **zoom,grid:** fix grid line pos during zoom ([e84a4f1](e84a4f1)), closes [#2156](#2156)

* **all:** contain inline css prop setting ([fde6a89](fde6a89)), closes [#2076](#2076)
* **api:** Intent to ship append load ([8076795](8076795)), closes [#2140](#2140)
* **data:** Intent to ship data.onshown/onhidden ([af98eb7](af98eb7)), closes [#2146](#2146)
* **data.labels:** Intent to ship data.labels.backgroundColors ([e0b2fed](e0b2fed)), closes [#1954](#1954)
* **subchart:** Intent to ship subchart.init.range option ([967bf1b](967bf1b)), closes [#2037](#2037)
* **subchart:** Intent to ship subchart.showHandle ([219bff3](219bff3)), closes [#2044](#2044)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants