feat(): includeDefaultValues arg

[ShaMan123]

Support accepting includeDefaultValues as an argument to toObject methods instead of fussing with object.includeDefaultValues

I have been wanting this for a long time
I think we should drop includeDefaultValues as a prop on the object/canvas level
And I think excludeFromExport can go away as well if we accept a filter function

toObject(props, includeDefaultValues, filter) {
// for each object
filter(obj) ? 'include': 'exclude'
}

[asturur]

Shachar the PR descriptions do not match up the quality and quantity of work you are doing.
To understand what you want to do i need to read the code, and that ( because how my brain works ) makes me distracted with the code instead of the purpose of the code.

I did not ready the code yet, i would like the default behaviour was to not include default values all the time, because that creates enormous object dumps that are often not useful.
It also makes the prototype changes less effective when restoring such object states.

includeDefaultValues for me can be either at canvas level, library level, i agree that at object level is not that much useful.
It can also be an argument of the function toObject as you did, that is good too.
I'm ok dumping it as a property.

In general i prefer a single argument in the form of object compared to many arguments, with modern js is easier to handle.

[github-actions]

Code Coverage Summary

> fabric@5.1.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   83.26 |    76.75 |   86.56 |   82.99 |                                               
 fabric.js |   83.26 |    76.75 |   86.56 |   82.99 | ...,29692,29817,29897-29962,30085,30184,30401 
-----------|---------|----------|---------|---------|-----------------------------------------------

[ShaMan123]

@asturur I think includeDefaultValues should be false as default as well. It's very strange to put default as true.
How to move forward? Shall we dump this property? I think it's entirely redundant now because once you call toObject you simply pass true as follows toObject(undefined, true)
Sorry for not explaining, will do next time.

[ShaMan123]

In general i prefer a single argument in the form of object compared to many arguments, with modern js is easier to handle.

You mean toObject({ includeDefaultValues: true, propertiesToInclude: ['stroke'] })?
That's good

[asturur]

In general i prefer a single argument in the form of object compared to many arguments, with modern js is easier to handle.

You mean toObject({ includeDefaultValues: true, propertiesToInclude: ['stroke'] })? That's good

Yes but when we have es6/TS we can think of change this shape.

[asturur]

For me you can move forward.
let's keep the separated arguments, get rid of the property everywhere, and let's have it as a false as default.
That will also avoid to need to check if is boolean or undefined.

[github-actions]

Code Coverage Summary

> fabric@5.1.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   83.31 |    76.79 |   86.56 |   83.04 |                                               
 fabric.js |   83.31 |    76.79 |   86.56 |   83.04 | ...,29692,29817,29897-29962,30085,30184,30401 
-----------|---------|----------|---------|---------|-----------------------------------------------

[ShaMan123]

@asturur what type of object is this?

fabric.js/src/filters/blendimage_filter.class.js

Line 225 in fd1b0c3

image: this.image && this.image.toObject(),

I am trying to figure out if to pass down the serialization args to it as well

[ShaMan123]

Finally all tests are passing. It was horrible...

[github-actions]

Code Coverage Summary

> fabric@5.1.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   83.27 |     76.7 |   86.55 |      83 |                                               
 fabric.js |   83.27 |     76.7 |   86.55 |      83 | ...,29654,29779,29859-29924,30047,30146,30363 
-----------|---------|----------|---------|---------|-----------------------------------------------

[github-actions]

Code Coverage Summary

> fabric@5.1.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   83.21 |    76.63 |   86.55 |   82.94 |                                               
 fabric.js |   83.21 |    76.63 |   86.55 |   82.94 | ...,29654,29779,29859-29924,30047,30146,30363 
-----------|---------|----------|---------|---------|-----------------------------------------------

[github-actions]

Code Coverage Summary

> fabric@5.1.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   83.27 |     76.7 |   86.55 |      83 |                                               
 fabric.js |   83.27 |     76.7 |   86.55 |      83 | ...,29654,29779,29859-29924,30047,30146,30363 
-----------|---------|----------|---------|---------|-----------------------------------------------

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[ShaMan123]

Will die out if/once removeDefaultValues is gone