Skip to content

fix(chart) :: group points into series without Object.prototype - #1449

Merged
lovasoa merged 1 commit into
mainfrom
81reap/chart-prototype-lookup
Sep 14, 2026
Merged

lovasoa merged 1 commit into
mainfrom
81reap/chart-prototype-lookup

Conversation

@81reap

@81reap 81reap commented Sep 12, 2026

Copy link
Copy Markdown
Collaborator

Points were grouped in a plain object keyed by the series value. A series named toString or constructor threw, rendering nothing. Numeric series names were enumerated in ascending order ahead of text ones rather than keeping the order the rows came in.

@81reap
81reap marked this pull request as ready for review September 12, 2026 21:19
@81reap
81reap requested a review from lovasoa September 12, 2026 21:21
@lovasoa

lovasoa commented Sep 12, 2026

Copy link
Copy Markdown
Collaborator

I agree the generated code is cleaner, but I'm not sure I understand the bug. Is there a real bug ?

If the only way to reproduce the "bug" is with somthing like

SELECT 'chart' AS component, 'constructor' AS type; -- constructor is not a valid chart type, this is documented
WITH points(series, x, y, color) AS (VALUES ('A', 'Q2', 2, 'toString')) SELECT * FROM points; -- toString is not a valid color, documented too

then I don't think this is a bug at all. You can rename it to a refactor or a code cleanup, and not mention it in the changelog at all.

Comment thread CHANGELOG.md Outdated
Points were grouped in a plain object keyed by the `series` value. A series named `toString` or `constructor` threw, rendering
nothing. Numeric series names were enumerated in ascending order ahead of text ones rather than keeping the order the rows came in.
@81reap
81reap force-pushed the 81reap/chart-prototype-lookup branch from 44309cd to 80aa3ab Compare September 13, 2026 01:10
@81reap

81reap commented Sep 13, 2026

Copy link
Copy Markdown
Collaborator Author

This was discovered as a side effect of #1448 but it didn't make sense to belong in that stack. Your right tho that this PR is doing too much and not all of this is an SQLPage issue. I have upstreamed apexcharts/apexcharts.js#5308 and apexcharts/apexcharts.js#5309 as well as updated this PR to focus only on the bug in SQLPage as well as updated the examples to be more clear.

@81reap 81reap changed the title fix(chart) :: look up colors and types without Object.prototype fix(chart) :: group points into series without Object.prototype Sep 13, 2026
@81reap
81reap requested a review from lovasoa September 13, 2026 01:35
@lovasoa

lovasoa commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

Good job!

@lovasoa
lovasoa merged commit 2cb3d02 into main Sep 14, 2026
51 checks passed
@lovasoa
lovasoa deleted the 81reap/chart-prototype-lookup branch September 14, 2026 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants