Skip to content

Commit

Permalink
Merge pull request #102 from ckeditor/i/98-mentions-throwing
Browse files Browse the repository at this point in the history
Fix: The inspector should not throw when attributes are objects with circular references. Closes #98.
  • Loading branch information
jodator authored Oct 26, 2020
2 parents a63bdea + dab8117 commit 36bf877
Show file tree
Hide file tree
Showing 5 changed files with 616 additions and 543 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
"eslint-plugin-react": "^7.12.4",
"husky": "^1.3.1",
"istanbul-instrumenter-loader": "^3.0.1",
"javascript-stringify": "^2.0.1",
"karma": "^4.0.0",
"karma-chai": "^0.1.0",
"karma-chrome-launcher": "^2.2.0",
Expand Down
16 changes: 15 additions & 1 deletion src/components/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
* For licensing, see LICENSE.md.
*/

import { stringify as javascriptStringify } from 'javascript-stringify';

export function stringify( value, quotesAroundText = true ) {
if ( value === undefined ) {
return 'undefined';
Expand All @@ -12,7 +14,9 @@ export function stringify( value, quotesAroundText = true ) {
return 'function() {…}';
}

const stringified = JSON.stringify( value );
const stringified = javascriptStringify( value, stringifySingleToDoubleQuotesReplacer, null, {
maxDepth: 1
} );

// Note: Remove leading and trailing quotes (") from the output. By default it is:
//
Expand Down Expand Up @@ -57,3 +61,13 @@ export function truncateString( string, length ) {

return string;
}

// Unlike JSON.stringify(), javascript-stringify returns single quotes around text.
// Retain the JSON.stringify() formatting instead of fixing 100 tests across the project :)
function stringifySingleToDoubleQuotesReplacer( value, indent, stringify ) {
if ( typeof value === 'string' ) {
return `"${ value.replace( '\'', '"' ) }"`;
}

return stringify( value );
}
2 changes: 1 addition & 1 deletion tests/inspector/ckeditorinspector.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ describe( 'CKEditorInspector', () => {

expect( wrapper.tagName.toLowerCase() ).to.equal( 'div' );
expect( wrapper.parentNode ).to.equal( document.body );
expect( wrapper.childNodes.length ).to.equal( 2 );
expect( wrapper.childNodes.length ).to.equal( 1 );
expect( wrapper.firstChild.classList.contains( 'ck-inspector' ) ).to.be.true;
} );

Expand Down
24 changes: 22 additions & 2 deletions tests/inspector/components/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe( 'Utils', () => {
expect( stringify( true ) ).to.equal( 'true' );
expect( stringify( 'foo' ) ).to.equal( '"foo"' );
expect( stringify( [ 'a' ] ) ).to.equal( '["a"]' );
expect( stringify( { a: false } ) ).to.equal( '{"a":false}' );
expect( stringify( { a: false } ) ).to.equal( '{a:false}' );
expect( stringify( () => 'foo' ) ).to.equal( 'function() {…}' );
} );

Expand All @@ -26,9 +26,29 @@ describe( 'Utils', () => {
expect( stringify( true ) ).to.equal( 'true' );
expect( stringify( 'foo', false ) ).to.equal( 'foo' );
expect( stringify( [ 'a' ], false ) ).to.equal( '["a"]' );
expect( stringify( { a: false }, false ) ).to.equal( '{"a":false}' );
expect( stringify( { a: false }, false ) ).to.equal( '{a:false}' );
expect( stringify( () => 'foo' ), false ).to.equal( 'function() {…}' );
} );

it( 'should not throw while processing circular references', () => {
const obj = { foo: 'bar' };
obj.baz = obj;

expect( () => {
expect( stringify( obj ) ).to.equal( '{foo:"bar"}' );
} ).to.not.throw();
} );

it( 'should process only the very first level of objects', () => {
const obj = {
level: '1',
nested: {
level: '2'
}
};

expect( stringify( obj ) ).to.equal( '{level:"1",nested:{}}' );
} );
} );

describe( 'uid()', () => {
Expand Down
Loading

0 comments on commit 36bf877

Please sign in to comment.