This repository has been archived by the owner on Jun 26, 2020. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 12
I/4992: Added support for creating a Collection
with initial items
#309
Open
oleq
wants to merge
8
commits into
master
Choose a base branch
from
i/4992
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
1aac49d
Added support for creating a Collection with initial items and supply…
oleq 61f59f4
Tests: Added a new test to check if #add is fired for all new items a…
oleq ef1870c
Docs: Documented private method in the Collection class.
oleq 8216550
Tests: Re-enabled all package tests.
oleq 4a04336
Merge branch 'master' into t/221
oleq 798a434
Code refactoring and test improvements.
oleq 1b7f953
Code refactoring.
oleq 28b9017
Docs: Improved Collection docs.
oleq File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -28,17 +28,54 @@ export default class Collection { | |||||
/** | ||||||
* Creates a new Collection instance. | ||||||
* | ||||||
* @param {Object} [options={}] The options object. | ||||||
* @param {String} [options.idProperty='id'] The name of the property which is considered to identify an item. | ||||||
* You can provide an array of initial items the collection will be created with: | ||||||
* | ||||||
* const collection = new Collection( [ { id: 'John' }, { id: 'Mike' } ] ); | ||||||
* | ||||||
* console.log( collection.get( 0 ) ); // -> { id: 'John' } | ||||||
* console.log( collection.get( 1 ) ); // -> { id: 'Mike' } | ||||||
* console.log( collection.get( 'Mike' ) ); // -> { id: 'Mike' } | ||||||
* | ||||||
* Or you can first create a collection and then add new items using the {@link #add} method: | ||||||
* | ||||||
* const collection = new Collection(); | ||||||
* | ||||||
* collection.add( { id: 'John' } ); | ||||||
* console.log( collection.get( 0 ) ); // -> { id: 'John' } | ||||||
* | ||||||
* Whatever option you choose, you can always pass a configuration object as the last argument | ||||||
* of the constructor: | ||||||
* | ||||||
* const emptyCollection = new Collection( { idProperty: 'name' } ); | ||||||
* emptyCollection.add( { name: 'John' } ); | ||||||
* console.log( collection.get( 'John' ) ); // -> { name: 'John' } | ||||||
* | ||||||
* const nonEmptyCollection = new Collection( [ { name: 'John' } ], { idProperty: 'name' } ); | ||||||
* nonEmptyCollection.add( { name: 'George' } ); | ||||||
* console.log( collection.get( 'George' ) ); // -> { name: 'George' } | ||||||
* | ||||||
* @param {Array.<Object>|Object} [initialItemsOrOptions] The initial items of the collection or | ||||||
* the options object. | ||||||
* @param {Object} [options={}] The options object, when the first argument is an array of initial items. | ||||||
* @param {String} [options.idProperty='id'] The name of the property which is used to identify an item. | ||||||
* Items that do not have such a property will be assigned one when added to the collection. | ||||||
*/ | ||||||
constructor( options = {} ) { | ||||||
constructor( initialItemsOrOptions = {}, options = {} ) { | ||||||
let initialItems = []; | ||||||
|
||||||
if ( initialItemsOrOptions instanceof Array ) { | ||||||
initialItems = initialItemsOrOptions; | ||||||
} else { | ||||||
options = initialItemsOrOptions; | ||||||
} | ||||||
|
||||||
/** | ||||||
* The internal list of items in the collection. | ||||||
* | ||||||
* @private | ||||||
* @member {Object[]} | ||||||
*/ | ||||||
this._items = []; | ||||||
this._items = [ ...initialItems ]; | ||||||
|
||||||
/** | ||||||
* The internal map of items in the collection. | ||||||
|
@@ -88,6 +125,11 @@ export default class Collection { | |||||
*/ | ||||||
this._skippedIndexesFromExternal = []; | ||||||
|
||||||
// Set the initial content of the collection (if provided in the constructor). | ||||||
for ( const item of initialItems ) { | ||||||
this._itemMap.set( this._getItemIdBeforeAdding( item ), item ); | ||||||
} | ||||||
|
||||||
/** | ||||||
* A collection instance this collection is bound to as a result | ||||||
* of calling {@link #bindTo} method. | ||||||
|
@@ -125,61 +167,72 @@ export default class Collection { | |||||
} | ||||||
|
||||||
/** | ||||||
* Adds an item into the collection. | ||||||
* Adds an item or multiple items into the collection. | ||||||
* | ||||||
* const collection = new Collection(); | ||||||
* | ||||||
* collection.add( { id: 'John' } ); | ||||||
* collection.add( { id: 'Anna' }, { id: 'George' } ); | ||||||
* | ||||||
* console.log( collection.length ); // -> 3 | ||||||
* console.log( collection.get( 2 ) ); // -> { name: 'George' } | ||||||
* | ||||||
* You can specify the index of the item (or items) when adding to the collection: | ||||||
* | ||||||
* If the item does not have an id, then it will be automatically generated and set on the item. | ||||||
* const collection = new Collection(); | ||||||
* | ||||||
* collection.add( { id: 'John' } ); | ||||||
* collection.add( { id: 'Bob' }, 0 ); | ||||||
* collection.add( { id: 'Anna' }, { id: 'George' }, 1 ); | ||||||
* | ||||||
* console.log( collection.get( 0 ) ); // -> { name: 'Bob' } | ||||||
* console.log( collection.get( 1 ) ); // -> { name: 'Anna' } | ||||||
* console.log( collection.get( 2 ) ); // -> { name: 'George' } | ||||||
* console.log( collection.get( 3 ) ); // -> { name: 'John' } | ||||||
* | ||||||
* **Note**: If an item does not have an id, it will be automatically given one (see {@link #constructor}). | ||||||
* | ||||||
* @chainable | ||||||
* @param {Object} item | ||||||
* @param {Number} [index] The position of the item in the collection. The item | ||||||
* is pushed to the collection when `index` not specified. | ||||||
* @param {...(Object)} items Item or items to be added to the collection. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
* @param {Number} [index] The position of the item (or items) when added to the collection. If not specified, | ||||||
* the item (or items) is pushed to the collection. | ||||||
* @fires add | ||||||
*/ | ||||||
add( item, index ) { | ||||||
let itemId; | ||||||
const idProperty = this._idProperty; | ||||||
add( ...args ) { | ||||||
let addIndex = args[ args.length - 1 ]; | ||||||
const items = args; | ||||||
|
||||||
if ( ( idProperty in item ) ) { | ||||||
itemId = item[ idProperty ]; | ||||||
|
||||||
if ( typeof itemId != 'string' ) { | ||||||
// E.g. add( { ... }, { ... }, ..., 3 ) | ||||||
if ( typeof addIndex === 'number' ) { | ||||||
if ( addIndex > this._items.length || addIndex < 0 ) { | ||||||
/** | ||||||
* This item's id should be a string. | ||||||
* The index number has invalid value. | ||||||
* | ||||||
* @error collection-add-invalid-id | ||||||
* @error collection-add-item-bad-index | ||||||
* @param {Number} index The index at which the item is to be added. | ||||||
* @param {module:utils/collection~Collection} collection The collection the item is added to. | ||||||
*/ | ||||||
throw new CKEditorError( 'collection-add-invalid-id', this ); | ||||||
throw new CKEditorError( 'collection-add-item-invalid-index', { | ||||||
index: addIndex, | ||||||
collection: this | ||||||
} ); | ||||||
} | ||||||
|
||||||
if ( this.get( itemId ) ) { | ||||||
/** | ||||||
* This item already exists in the collection. | ||||||
* | ||||||
* @error collection-add-item-already-exists | ||||||
*/ | ||||||
throw new CKEditorError( 'collection-add-item-already-exists', this ); | ||||||
} | ||||||
} else { | ||||||
item[ idProperty ] = itemId = uid(); | ||||||
// The last argument was an index, remove it from the items. | ||||||
items.pop(); | ||||||
} | ||||||
|
||||||
// TODO: Use ES6 default function argument. | ||||||
if ( index === undefined ) { | ||||||
index = this._items.length; | ||||||
} else if ( index > this._items.length || index < 0 ) { | ||||||
/** | ||||||
* The index number has invalid value. | ||||||
* | ||||||
* @error collection-add-item-bad-index | ||||||
*/ | ||||||
throw new CKEditorError( 'collection-add-item-invalid-index', this ); | ||||||
// E.g. add( { ... }, { ... } ) | ||||||
else { | ||||||
addIndex = this._items.length; | ||||||
} | ||||||
|
||||||
this._items.splice( index, 0, item ); | ||||||
this._items.splice( addIndex, 0, ...items ); | ||||||
|
||||||
this._itemMap.set( itemId, item ); | ||||||
|
||||||
this.fire( 'add', item, index ); | ||||||
items.forEach( ( item, itemIndex ) => { | ||||||
this._itemMap.set( this._getItemIdBeforeAdding( item ), item ); | ||||||
this.fire( 'add', item, addIndex + itemIndex ); | ||||||
} ); | ||||||
|
||||||
return this; | ||||||
} | ||||||
|
@@ -604,6 +657,55 @@ export default class Collection { | |||||
} ); | ||||||
} | ||||||
|
||||||
/** | ||||||
* For a given item to be added to the collection, it validates item's "id" property or generates | ||||||
* a new one if missing. If the validation was successful or a new id was generated, | ||||||
* the id is returned by the method. Otherwise, an error is thrown. | ||||||
* | ||||||
* @private | ||||||
* @param {Object} item An item to be added to the collection. | ||||||
*/ | ||||||
_getItemIdBeforeAdding( item ) { | ||||||
const idProperty = this._idProperty; | ||||||
let itemId; | ||||||
|
||||||
if ( ( idProperty in item ) ) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unnecessary parentheses. |
||||||
itemId = item[ idProperty ]; | ||||||
|
||||||
if ( typeof itemId != 'string' ) { | ||||||
/** | ||||||
* This item's id should be a string. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
* | ||||||
* @error collection-add-invalid-id | ||||||
* @param {Object} item The item being added to the collection. | ||||||
* @param {module:utils/collection~Collection} collection The collection the item is added to. | ||||||
*/ | ||||||
throw new CKEditorError( 'collection-add-invalid-id', { | ||||||
item, | ||||||
collection: this, | ||||||
} ); | ||||||
} | ||||||
|
||||||
if ( this.get( itemId ) ) { | ||||||
/** | ||||||
* This item already exists in the collection. | ||||||
* | ||||||
* @error collection-add-item-already-exists | ||||||
* @param {Object} item The item being added to the collection. | ||||||
* @param {module:utils/collection~Collection} collection The collection the item is added to. | ||||||
*/ | ||||||
throw new CKEditorError( 'collection-add-item-already-exists', { | ||||||
item, | ||||||
collection: this | ||||||
} ); | ||||||
} | ||||||
} else { | ||||||
item[ idProperty ] = itemId = uid(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not a fan of double assignments. Maybe move the |
||||||
} | ||||||
|
||||||
return itemId; | ||||||
} | ||||||
|
||||||
/** | ||||||
* Iterable interface. | ||||||
* | ||||||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to see constructor accepting more generic,
Iterable
compliant values. This would allow usingSet
s too.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Me too :D