Skip to content

Fix TypeScript typing for filterColor shader hook#8644

Open
Kathrina-dev wants to merge 13 commits intoprocessing:dev-2.0from
Kathrina-dev:fix-filterColor-object-type
Open

Fix TypeScript typing for filterColor shader hook#8644
Kathrina-dev wants to merge 13 commits intoprocessing:dev-2.0from
Kathrina-dev:fix-filterColor-object-type

Conversation

@Kathrina-dev
Copy link

@Kathrina-dev Kathrina-dev commented Mar 15, 2026

Resolves #8574

Changes

Fixes incorrect TypeScript typing for the filterColor shader hook.

Previously, the generated TypeScript definitions declared:

declare const filterColor: object;

Because of this, accessing documented properties such as:

  • filterColor.texCoord
  • filterColor.canvasSize
  • filterColor.begin()
  • filterColor.set(...)

resulted in TypeScript errors like:

Property 'texCoord' does not exist on type 'object'.

Other Methods attempted at resolving the error

Initially I attempted to resolve this through JSDoc annotations in the source by using:

  • @property
  • @typedef

to explicitly define the structure of filterColor.

However, the documentation generator used in the build pipeline still produced the type as object in the generated TypeScript definitions. Because of this limitation in the JSDoc → TypeScript generation step, the correct structure was not preserved.

Solution

To ensure the generated typings match the documented API, I added a patch in:

utils/patch.mjs

This patch updates the generated declaration to:

declare const filterColor: {
texCoord: any;
canvasSize: any;
texelSize: any;
canvasContent: any;
begin(): void;
end(): void;
set(color: any): void;
};

This aligns the TypeScript definitions with the documented properties of the filterColor shader hook and prevents TypeScript errors when accessing them.

Screenshots of the change

N/A (TypeScript typing fix)

Suggested Reviewers

PR Checklist

  • npm run lint passes
  • Inline reference is included / updated
  • Unit tests are included / updated

@Kathrina-dev Kathrina-dev force-pushed the fix-filterColor-object-type branch from 35fba48 to bd57864 Compare March 15, 2026 06:43
@Kathrina-dev Kathrina-dev changed the base branch from main to dev-2.0 March 15, 2026 06:47
@Kathrina-dev Kathrina-dev force-pushed the fix-filterColor-object-type branch from bd57864 to a5f85a7 Compare March 15, 2026 06:50
Copy link
Contributor

@davepagurek davepagurek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking this on @Kathrina-dev! While the original issue was for filterColor, I think this actually applies to all the hook objects documented around filterColor in the code. While we could add a patch method for each one, this seems likely to drift from the documented properties, especially as we add new hooks. It might be worth instead taking the approach of updating the typescript generation script to work with JSDoc typedefs (or something else, like documenting a pretend class for each hook object or something?) so that we can have the jsdoc be the sole source of truth for both documentation and types.

@Kathrina-dev
Copy link
Author

Thanks for the suggestion, I've already removed the formatted part of the code to make it easier to identify the diff.

My initial approach was to use Typescript generation script (@Property, @typedef) but it kept collapsing the JSDoc typedef into an object. I agree that adding a patch would cause a drift from the documented properties, so I'll try the alternative way you've suggested and let you know the results.

@davepagurek
Copy link
Contributor

Hi! If you run npm run test:types, it gets a number of errors, e.g.:

Error: types/global.d.ts(1387,21): error TS2749: 'CENTER' refers to a value, but is being used as a type here. Did you mean 'typeof CENTER'?
Error: types/global.d.ts(1387,30): error TS2749: 'RADIUS' refers to a value, but is being used as a type here. Did you mean 'typeof RADIUS'?
Error: types/global.d.ts(1387,39): error TS2749: 'CORNER' refers to a value, but is being used as a type here. Did you mean 'typeof CORNER'?
Error: types/global.d.ts(1387,48): error TS2749: 'CORNERS' refers to a value, but is being used as a type here. Did you mean 'typeof CORNERS'?
Error: types/global.d.ts(1574,36): error TS2749: 'FALLBACK' refers to a value, but is being used as a type here. Did you mean 'typeof FALLBACK'?
Error: types/global.d.ts(1574,47): error TS2749: 'LABEL' refers to a value, but is being used as a type here. Did you mean 'typeof LABEL'?
Error: types/global.d.ts(1636,60): error TS2749: 'P2D' refers to a value, but is being used as a type here. Did you mean 'typeof P2D'?
Error: types/global.d.ts(1636,66): error TS2749: 'WEBGL' refers to a value, but is being used as a type here. Did you mean 'typeof WEBGL'?
Error: types/global.d.ts(1636,74): error TS2749: 'P2DHDR' refers to a value, but is being used as a type here. Did you mean 'typeof P2DHDR'?
Error: types/global.d.ts(1670,24): error TS2749: 'FALLBACK' refers to a value, but is being used as a type here. Did you mean 'typeof FALLBACK'?
Error: types/global.d.ts(1670,35): error TS2749: 'LABEL' refers to a value, but is being used as a type here. Did you mean 'typeof LABEL'?
Error: types/global.d.ts(1704,136): error TS2749: 'BLEND' refers to a value, but is being used as a type here. Did you mean 'typeof BLEND'?
Error: types/global.d.ts(1704,144): error TS2749: 'DARKEST' refers to a value, but is being used as a type here. Did you mean 'typeof DARKEST'?
Error: types/global.d.ts(1704,154): error TS2749: 'LIGHTEST' refers to a value, but is being used as a type here. Did you mean 'typeof LIGHTEST'?
Error: types/global.d.ts(1704,165): error TS2749: 'DIFFERENCE' refers to a value, but is being used as a type here. Did you mean 'typeof DIFFERENCE'?
Error: types/global.d.ts(1704,178): error TS2749: 'MULTIPLY' refers to a value, but is being used as a type here. Did you mean 'typeof MULTIPLY'?
Error: types/global.d.ts(1704,189): error TS2749: 'EXCLUSION' refers to a value, but is being used as a type here. Did you mean 'typeof EXCLUSION'?

Looks like the way the existing typedefs are being exported has changed -- the typedef itself should be a type, but also there should be a const of the same name from the rest of the jsdoc.

@Kathrina-dev
Copy link
Author

I think the issue was that some constants (like BLEND, ADD, etc.) weren’t being picked up properly. I updated the lookup to include them and adjusted how they’re handled in the type conversion depending on where they’re used. That seems to fix the errors.

Thanks for being patient with me, I'm new to contributing to Open Source and forgot to run the test cases before committing. I'll keep it in mind next time.

Copy link
Contributor

@davepagurek davepagurek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making some updates, good to see the tests passing now! It looks like although they past, the generated type files still aren't 100% correct. I left some comments, one of which includes some steps I'd follow when working on this to help verify the change.

* @property {any} canvasSize
* @property {any} texelSize
* @property {any} canvasContent
* @property {function(): undefined} begin
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these be : void too or is there a reason for them to be : undefined?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I tried using void, the generated p5.d.ts ended up showing any instead. I tried using different ways to define a function but it still kept showing any. So I've defined it as undefined which showed up correctly while generating p5.js.d.ts

@Kathrina-dev
Copy link
Author

Thanks for taking out the time to leave those reviews, I'll look further into everything you've mentioned. This issue is a lot bigger than I expected but I'm very thankful for your help in this matter.

@Kathrina-dev
Copy link
Author

Kathrina-dev commented Mar 19, 2026

Considering that this issue requires alot more modifications to the convertTypeToTypeScript and generateTypeDefinitions functions rather than just a small type fix issue.

I'm considering to revert the changes back to how it was before and then creating a seperate function to handle Object types parsing. Since we already have separate functions for classes and methods, I think doing the same for typedefs will make it easier to handle things like function properties and avoid affecting other type conversions. This will make the convertTypeToTypeScript and generateTypeDefinitions less cluttered and make it modular.

What are your thoughts on this approach @davepagurek ?

@davepagurek
Copy link
Contributor

That sounds ok to me @Kathrina-dev! Agreed that this gets a little hairy, thanks for your patience working on it!

@Kathrina-dev
Copy link
Author

Kathrina-dev commented Mar 20, 2026

@davepagurek I've added the new function to handle typedef properties. It passes the npm run test:types locally and I've also compared it to the generated types from types/p5.d.ts in the dev-2.0 branch and everything matches up exactly + the new introduced type interface.

The only concern I have is about leaving the begin() and end() function as "undefined". I've tried to type it as void but after generating, it shows up as "any". I've made a temporary fix by parsing "undefined" and converting it to "void" in the hasTypedefProperties function but I'm not sure that's the correct approach. How could we best fix this issue?

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.

[p5.js 2.0 Bug Report]: p5.strands | methods and properties of filterColor not known in instance mode

2 participants