diff --git a/CHANGELOG.md b/CHANGELOG.md index a1caf48d..c34adad8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,13 +7,14 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### Added * Show a warning when a stage has too many layers. +* Show a warning on duplicate ids ### Changed * Fixes inconsistent `layer.setSize()` method. Now it has same arguments as any container. * Full rewrite to Typescript with tons of refactoring and small optimizations. The public API should be 100% the same * Fixed `patternImage` and `radialGradient` for `Konva.Text` * `Konva.Util._isObject` is renamed to `Konva.Util._isPlainObject`. -* TODO: changed behavior of `removeId`. +* changed behavior of `removeId`. ### Removed * `Konva.Util.addMethods` diff --git a/konva.js b/konva.js index 2ddbf7aa..423e6d7c 100644 --- a/konva.js +++ b/konva.js @@ -20,8 +20,6 @@ * @namespace Konva */ var version = '@@version'; - // private - var ids = {}; var names = {}; var shapes = {}; var isBrowser = typeof window !== 'undefined' && @@ -62,31 +60,6 @@ } return false; }; - var _addId = function (node, id) { - if (!id) { - return; - } - // do we need this warning? - // if (ids[id]) { - // console.warn( - // 'Duplicate id "' + - // id + - // '". Please don not use same id several times. It may break find() method look up.' - // ); - // } - ids[id] = node; - }; - var _removeId = function (id, node) { - // node has no id - if (!id) { - return; - } - // another node is registered (possible for duplicate ids) - if (ids[id] !== node) { - return; - } - delete ids[id]; - }; var _addName = function (node, name) { if (name) { if (!names[name]) { @@ -2086,6 +2059,28 @@ return HitCanvas; }(Canvas)); + var ids = {}; + var ID_WARNING = "Duplicate id \"{id}\".\nPlease do not use same id several times, it will break find() method look up.\nIf you have duplicates it is better to use \"name\" property instead.\n"; + var _addId = function (node, id) { + if (!id) { + return; + } + if (ids[id]) { + Util.warn(ID_WARNING.replace('{id}', id)); + } + ids[id] = node; + }; + var _removeId = function (id, node) { + // node has no id + if (!id) { + return; + } + // another node is registered (possible for duplicate ids) + if (ids[id] !== node) { + return; + } + delete ids[id]; + }; // CONSTANTS var ABSOLUTE_OPACITY = 'absoluteOpacity', ABSOLUTE_TRANSFORM = 'absoluteTransform', ABSOLUTE_SCALE = 'absoluteScale', CHANGE = 'Change', CHILDREN = 'children', KONVA = 'konva', LISTENING = 'listening', MOUSEENTER = 'mouseenter', MOUSELEAVE = 'mouseleave', NAME = 'name', SET$1 = 'set', SHAPE = 'Shape', SPACE = ' ', STAGE = 'stage', TRANSFORM = 'transform', UPPER_STAGE = 'Stage', VISIBLE = 'visible', CLONE_BLACK_LIST = ['id'], TRANSFORM_CHANGE_STR = [ 'xChange.konva', @@ -15650,6 +15645,7 @@ Collection: Collection, Util: Util, Node: Node, + ids: ids, Container: Container, Stage: Stage, stages: stages, @@ -15680,7 +15676,6 @@ Transformer: Transformer, Wedge: Wedge, version: version, - ids: ids, names: names, shapes: shapes, isBrowser: isBrowser, @@ -15688,8 +15683,6 @@ dblClickWindow: dblClickWindow, isDragging: isDragging, isDragReady: isDragReady, - _addId: _addId, - _removeId: _removeId, _addName: _addName, _removeName: _removeName, getAngle: getAngle, diff --git a/src/Container.ts b/src/Container.ts index 24fdf40f..aa86d5aa 100644 --- a/src/Container.ts +++ b/src/Container.ts @@ -1,9 +1,9 @@ import { Util, Collection } from './Util'; import { Factory, Validators } from './Factory'; -import { Node } from './Node'; -import { getGlobalKonva, ids, names } from './Global'; +import { Node, ids } from './Node'; +import { getGlobalKonva, names } from './Global'; -import { GetSet, Vector2d, IRect } from './types'; +import { GetSet, IRect } from './types'; /** * Container constructor.  Containers are used to contain nodes or other containers diff --git a/src/Global.ts b/src/Global.ts index c17f98a3..eeab52d2 100644 --- a/src/Global.ts +++ b/src/Global.ts @@ -15,8 +15,6 @@ var PI_OVER_180 = Math.PI / 180; */ export const version = '@@version'; -// private -export const ids = {}; export const names = {}; export const shapes = {}; @@ -66,32 +64,6 @@ export const isDragReady = function() { } return false; }; -export const _addId = function(node: any, id) { - if (!id) { - return; - } - // do we need this warning? - // if (ids[id]) { - // console.warn( - // 'Duplicate id "' + - // id + - // '". Please don not use same id several times. It may break find() method look up.' - // ); - // } - ids[id] = node; -}; - -export const _removeId = function(id: string, node: any) { - // node has no id - if (!id) { - return; - } - // another node is registered (possible for duplicate ids) - if (ids[id] !== node) { - return; - } - delete ids[id]; -}; export const _addName = function(node: any, name) { if (name) { diff --git a/src/Node.ts b/src/Node.ts index f1871c54..ffe96c37 100644 --- a/src/Node.ts +++ b/src/Node.ts @@ -1,16 +1,39 @@ import { Util, Collection, Transform, RectConf, Point } from './Util'; import { Factory, Validators } from './Factory'; import { SceneCanvas, HitCanvas } from './Canvas'; -import { - _removeId, - _removeName, - _addId, - _addName, - getGlobalKonva -} from './Global'; +import { _removeName, _addName, getGlobalKonva } from './Global'; import { Container } from './Container'; import { GetSet, Vector2d } from './types'; +export const ids = {}; + +const ID_WARNING = `Duplicate id "{id}". +Please do not use same id several times, it will break find() method look up. +If you have duplicates it is better to use "name" property instead. +`; + +const _addId = function(node: Node, id) { + if (!id) { + return; + } + if (ids[id]) { + Util.warn(ID_WARNING.replace('{id}', id)); + } + ids[id] = node; +}; + +export const _removeId = function(id: string, node: any) { + // node has no id + if (!id) { + return; + } + // another node is registered (possible for duplicate ids) + if (ids[id] !== node) { + return; + } + delete ids[id]; +}; + export type Filter = (this: Node, imageData: ImageData) => void; type globalCompositeOperationType = diff --git a/src/internals.ts b/src/internals.ts index f8fc11be..67a017f6 100644 --- a/src/internals.ts +++ b/src/internals.ts @@ -1,7 +1,7 @@ export * from './Global'; export { Collection, Util } from './Util'; -export { Node } from './Node'; +export { Node, ids } from './Node'; export { Container } from './Container'; export { Stage, stages } from './Stage'; diff --git a/test/unit/Node-test.js b/test/unit/Node-test.js index 5375f9be..b842300f 100644 --- a/test/unit/Node-test.js +++ b/test/unit/Node-test.js @@ -118,20 +118,23 @@ suite('Node', function() { assert.equal(layer.getAbsoluteOpacity(), 0.5); }); - test.skip('warn on duplicate id', function() { + test.only('warn on duplicate id', function() { var oldWarn = Konva.Util.warn; var called = false; Konva.Util.warn = function() { called = true; }; - var circle = new Konva.Circle({ + var circle1 = new Konva.Circle({ id: 'circle' }); - var circle = new Konva.Circle({ + var circle2 = new Konva.Circle({ id: 'circle' }); + assert.equal(called, true); Konva.Util.warn = oldWarn; + circle1.destroy(); + circle2.destroy(); }); // ====================================================== @@ -1153,8 +1156,6 @@ suite('Node', function() { layer.add(circle); stage.add(layer); - - /* * add custom attr that points to self. The setAttrs method should * not inifinitely recurse causing a stack overflow @@ -3085,7 +3086,7 @@ suite('Node', function() { // last shape is registered assert.equal(Konva.ids.shape, rect); - + // destroying circle should not remove rect from regiter circle.destroy(); diff --git a/test/unit/Stage-test.js b/test/unit/Stage-test.js index 6d63f760..7569e190 100644 --- a/test/unit/Stage-test.js +++ b/test/unit/Stage-test.js @@ -1231,12 +1231,11 @@ suite('Stage', function() { image.src = url; }); - test.only('show a warning if the stage has too many layers', function() { + test('show a warning if the stage has too many layers', function() { var stage = addStage(); var oldWarn = Konva.Util.warn; var called = false; Konva.Util.warn = function() { - oldWarn.apply(null, arguments); called = true; };