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

fix(): weird toJson bug #8111

Merged
merged 5 commits into from
Aug 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions .github/workflows/build.js.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,16 @@ jobs:
node-version: 16.x
- run: npm ci
- run: npm run build
lint:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Linting
uses: actions/setup-node@v1
with:
node-version: 16.x
- run: npm ci
- run: npm run lint
# lint:
# runs-on: ubuntu-latest
# steps:
# - uses: actions/checkout@v2
# - name: Linting
# uses: actions/setup-node@v1
# with:
# node-version: 16.x
# - run: npm ci
# - run: npm run lint
coverage:
runs-on: ubuntu-latest
steps:
Expand Down
5 changes: 2 additions & 3 deletions src/shapes/object.class.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1778,12 +1778,11 @@

/**
* Returns a JSON representation of an instance
* @param {Array} [propertiesToInclude] Any properties that you might want to additionally include in the output
* @return {Object} JSON
*/
toJSON: function(propertiesToInclude) {
toJSON: function() {
// delegate, not alias
return this.toObject(propertiesToInclude);
return this.toObject();
},

/**
Expand Down
39 changes: 20 additions & 19 deletions src/static_canvas.class.ts
Original file line number Diff line number Diff line change
Expand Up @@ -990,6 +990,26 @@
return this._toObjectMethod('toObject', propertiesToInclude);
},


/**
* Returns Object representation of canvas
* this alias is provided because if you call JSON.stringify on an instance,
* the toJSON object will be invoked if it exists.
* Having a toJSON method means you can do JSON.stringify(myCanvas)
* @return {Object} JSON compatible object
* @tutorial {@link http://fabricjs.com/fabric-intro-part-3#serialization}
* @see {@link http://jsfiddle.net/fabricjs/pec86/|jsFiddle demo}
* @example <caption>JSON without additional properties</caption>
* var json = canvas.toJSON();
* @example <caption>JSON with additional properties included</caption>
* var json = canvas.toJSON(['lockMovementX', 'lockMovementY', 'lockRotation', 'lockScalingX', 'lockScalingY']);
* @example <caption>JSON without default values</caption>
* var json = canvas.toJSON();
*/
toJSON: function() {
return this.toObject();
},

/**
* Returns dataless object representation of canvas
* @param {Array} [propertiesToInclude] Any properties that you might want to additionally include in the output
Expand Down Expand Up @@ -1688,25 +1708,6 @@
}
});

/**
* Returns Object representation of canvas
* this alias is provided because if you call JSON.stringify on an instance,
* the toJSON object will be invoked if it exists.
* Having a toJSON method means you can do JSON.stringify(myCanvas)
* @function
* @param {Array} [propertiesToInclude] Any properties that you might want to additionally include in the output
* @return {Object} JSON compatible object
* @tutorial {@link http://fabricjs.com/fabric-intro-part-3#serialization}
* @see {@link http://jsfiddle.net/fabricjs/pec86/|jsFiddle demo}
* @example <caption>JSON without additional properties</caption>
* var json = canvas.toJSON();
* @example <caption>JSON with additional properties included</caption>
* var json = canvas.toJSON(['lockMovementX', 'lockMovementY', 'lockRotation', 'lockScalingX', 'lockScalingY']);
* @example <caption>JSON without default values</caption>
* canvas.includeDefaultValues = false;
* var json = canvas.toJSON();
*/
fabric.StaticCanvas.prototype.toJSON = fabric.StaticCanvas.prototype.toObject;
Copy link
Member

Choose a reason for hiding this comment

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

this was probably my fault.

Copy link
Member

Choose a reason for hiding this comment

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

image

well not this time, but this looks totally like something i would do.

Copy link
Member

Choose a reason for hiding this comment

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

11 years this bug and no one was armed

Copy link
Member

Choose a reason for hiding this comment

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

then comes @ShaMan123 ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤪


if (fabric.isLikelyNode) {
fabric.StaticCanvas.prototype.createPNGStream = function() {
Expand Down
12 changes: 6 additions & 6 deletions test/unit/canvas_static.js
Original file line number Diff line number Diff line change
Expand Up @@ -1011,7 +1011,7 @@

QUnit.test('toJSON', function(assert) {
assert.ok(typeof canvas.toJSON === 'function');
assert.equal(JSON.stringify(canvas.toJSON()), '{"version":"' + fabric.version + '","objects":[]}');
assert.equal(JSON.stringify(canvas), '{"version":"' + fabric.version + '","objects":[]}');
canvas.backgroundColor = '#ff5555';
canvas.overlayColor = 'rgba(0,0,0,0.2)';
assert.equal(JSON.stringify(canvas.toJSON()), '{"version":"' + fabric.version + '","objects":[],"background":"#ff5555","overlay":"rgba(0,0,0,0.2)"}', '`background` and `overlay` value should be reflected in json');
Expand All @@ -1027,7 +1027,7 @@

canvas.bar = 456;

var data = canvas.toJSON(['padding', 'foo', 'bar', 'baz']);
var data = canvas.toObject(['padding', 'foo', 'bar', 'baz']);
assert.ok('padding' in data.objects[0]);
assert.ok('foo' in data.objects[0], 'foo shouldn\'t be included if it\'s not in an object');
assert.ok(!('bar' in data.objects[0]), 'bar shouldn\'t be included if it\'s not in an object');
Expand Down Expand Up @@ -1059,7 +1059,7 @@
createImageObject(function(image) {
canvas.backgroundImage = image;
image.custom = 'yes';
var json = canvas.toJSON(['custom']);
var json = canvas.toObject(['custom']);
assert.equal(json.backgroundImage.custom, 'yes');
canvas.backgroundImage = null;
done();
Expand Down Expand Up @@ -1088,7 +1088,7 @@
createImageObject(function(image) {
canvas.overlayImage = image;
image.custom = 'yes';
var json = canvas.toJSON(['custom']);
var json = canvas.toObject(['custom']);
assert.equal(json.overlayImage.custom, 'yes');
canvas.overlayImage = null;
done();
Expand Down Expand Up @@ -1351,8 +1351,8 @@

canvas.add(rect);

var jsonWithoutFoo = JSON.stringify(canvas.toJSON(['padding']));
var jsonWithFoo = JSON.stringify(canvas.toJSON(['padding', 'foo']));
var jsonWithoutFoo = JSON.stringify(canvas.toObject(['padding']));
var jsonWithFoo = JSON.stringify(canvas.toObject(['padding', 'foo']));

assert.equal(jsonWithFoo, RECT_JSON_WITH_PADDING);
assert.ok(jsonWithoutFoo !== RECT_JSON_WITH_PADDING);
Expand Down
1 change: 1 addition & 0 deletions test/unit/object.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@
var cObj = new fabric.Object();
assert.ok(typeof cObj.toJSON === 'function');
assert.equal(JSON.stringify(cObj.toJSON()), emptyObjectJSON);
assert.equal(JSON.stringify(cObj), emptyObjectJSON);

cObj.set('opacity', 0.88)
.set('scaleX', 1.3)
Expand Down