Skip to content

Conversation

Smrtnyk
Copy link
Contributor

@Smrtnyk Smrtnyk commented Apr 20, 2025

No description provided.

Copy link

codesandbox bot commented Apr 20, 2025

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

@@ -734,7 +734,7 @@ export class FabricObject<
*/
hasStroke() {
return (
this.stroke && this.stroke !== 'transparent' && this.strokeWidth !== 0
!!this.stroke && this.stroke !== 'transparent' && this.strokeWidth !== 0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

without this it would return empty string
although "falsy" it is not a boolean
qunit was testing this loosely treating empty string as false
but vitest is bit more strict and precise here

Copy link
Member

Choose a reason for hiding this comment

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

the intention of this is to be a boolean, can you add it to the typo of hasStroke explicitly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean to add :boolean as return type of this method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@asturur
Copy link
Member

asturur commented Apr 20, 2025

ok update changelog seem to work finally

Copy link
Contributor

Build Stats

file / KB (diff) bundled minified
fabric 913.867 (+0.004) 301.234 (+0.002)

Copy link
Contributor


> fabric@6.6.2 coverage:report:ci
> nyc report --reporter=text-summary


=============================== Coverage summary ===============================
Statements   : 75.67% ( 21749/28739 )
Branches     : 61.77% ( 4256/6890 )
Functions    : 86.19% ( 1555/1804 )
Lines        : 80.45% ( 16666/20715 )
================================================================================

@asturur asturur merged commit cee5f30 into fabricjs:master Apr 20, 2025
20 checks passed
@Smrtnyk Smrtnyk deleted the object-migration branch April 20, 2025 14:33
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