Skip to content

Commit

Permalink
Merge pull request #19 from canjs/push-event
Browse files Browse the repository at this point in the history
 fix for .push dispatching a length event
  • Loading branch information
matthewp committed Jun 14, 2019
2 parents a2e1c3a + 5c9371e commit 676cee4
Show file tree
Hide file tree
Showing 7 changed files with 212 additions and 152 deletions.
66 changes: 66 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
# Contributing to can-define-array

Check out the [contribution guide on CanJS.com](https://canjs.com/doc/guides/contribute.html) for information on:

- [Code of Conduct](https://canjs.com/doc/guides/contribute.html#CodeofConduct)
- [Getting Help](https://canjs.com/doc/guides/contribute.html#GettingHelp)
- [Project Organization](https://canjs.com/doc/guides/contributing/project-organization.html)
- [Reporting Bugs](https://canjs.com/doc/guides/contributing/bug-report.html)
- [Suggesting Features](https://canjs.com/doc/guides/contributing/feature-suggestion.html)
- [Finding Ways to Contribute](https://canjs.com/doc/guides/contributing/finding-ways-to-contribute.html)

The rest of this guide has information that’s specific to this repository.

## Developing Locally

This section will walk you through setting up the [repository](https://github.com/canjs/can-define-array) on your computer.

### Signing up for GitHub

If you don’t already have a GitHub account, you’ll need to [create a new one](https://help.github.com/articles/signing-up-for-a-new-github-account/).

### Forking & cloning the repository

A “fork” is a copy of a repository in your personal GitHub account. “Cloning” is the process of getting the repository’s source code on your computer.

GitHub has a guide for [forking a repo](https://help.github.com/articles/fork-a-repo/). To fork can-define-array, you can start by going to its [fork page](https://github.com/canjs/can-define-array/fork).

Next, you’ll want to clone the repo. [GitHub’s cloning guide](https://help.github.com/articles/cloning-a-repository/) explains how to do this on Linux, Mac, or Windows.

GitHub’s guide will [instruct you](https://help.github.com/articles/fork-a-repo/#step-2-create-a-local-clone-of-your-fork) to clone it with a command like:

```shell
git clone https://github.com/YOUR-USERNAME/can-define-array
```

Make sure you replace `YOUR-USERNAME` with your GitHub username.

### Installing the dependencies

After you’ve forked & cloned the repository, you’ll need to install the project’s dependencies.

First, make sure you’ve [installed Node.js and npm](https://docs.npmjs.com/getting-started/installing-node).

If you just cloned the repo from the command line, you’ll want to switch to the folder with your clone:

```shell
cd can-define-array
```

Next, install the project’s dependencies with npm:

```shell
npm install
```

### Running the tests

You can manually run this repository’s tests in any browser by starting the dev server (see the section above) and visiting this page: http://localhost:8080/test/test.html

Firefox is used to run the repository’s automated tests from the command line. If you don’t already have it, [download Firefox](https://www.mozilla.org/en-US/firefox/new/). Mozilla has guides for installing it on [Linux](https://support.mozilla.org/t5/Install-and-Update/Install-Firefox-on-Linux/ta-p/2516), [Mac](https://support.mozilla.org/t5/Install-and-Update/How-to-download-and-install-Firefox-on-Mac/ta-p/3453), and [Windows](https://support.mozilla.org/t5/Install-and-Update/How-to-download-and-install-Firefox-on-Windows/ta-p/2210).

After Firefox is installed, you can run:

```shell
npm test
```
9 changes: 9 additions & 0 deletions LICENSE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
The MIT License (MIT)

Copyright (c) 2019 Bitovi

Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
22 changes: 21 additions & 1 deletion readme.md
Original file line number Diff line number Diff line change
@@ -1,2 +1,22 @@
# can-define-array

# can-define-array WIP
[![Join our Slack](https://img.shields.io/badge/slack-join%20chat-611f69.svg)](https://www.bitovi.com/community/slack?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge)
[![Join our Discourse](https://img.shields.io/discourse/https/forums.bitovi.com/posts.svg)](https://forums.bitovi.com/?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge)
[![License: MIT](https://img.shields.io/badge/license-MIT-blue.svg)](https://github.com/canjs/can-define-array/blob/master/LICENSE.md)
[![npm version](https://badge.fury.io/js/can-define-array.svg)](https://www.npmjs.com/package/can-define-array)
[![Travis build status](https://travis-ci.com/canjs/can-define-array.svg?branch=master)](https://travis-ci.com/canjs/can-define-array)
[![Greenkeeper badge](https://badges.greenkeeper.io/canjs/can-define-array.svg)](https://greenkeeper.io/)

Import/require **can-define-array** and use the Element to derive your own classes. Calling `customElements.define` will register your element with the window's registry of custom elements.

## Documentation

Read the [can-define-array API docs on CanJS.com](https://v3.canjs.com/doc/can-define-arracan-define-array the [latest releases on GitHub](https://github.com/canjs/can-define-array/releases).

## Contributing

The [contribution guide](https://github.com/canjs/can-define-array/blob/master/CONTRIBUTING.md) has information on getting help, reporting bugs, developing locally, and more.

## License

[MIT](https://github.com/canjs/can-define-array/blob/master/LICENSE.md)
64 changes: 0 additions & 64 deletions src/can-define-array.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ const {
mixinMapProps,
mixinTypeEvents
} = require("can-define-mixin");
const ObservationRecorder = require("can-observation-recorder");
const ProxyArray = require("./proxy-array")();
const queues = require("can-queues");

Expand All @@ -25,60 +24,6 @@ function convertItems(Constructor, items) {
}
}

function triggerChange(attr, how, newVal, oldVal) {
var index = +attr;
// `batchTrigger` direct add and remove events...

// Make sure this is not nested and not an expando
if ( !isNaN(index)) {
var itemsDefinition = this._define.definitions["#"];
var patches, dispatched;
if (how === 'add') {
if (itemsDefinition && typeof itemsDefinition.added === 'function') {
ObservationRecorder.ignore(itemsDefinition.added).call(this, newVal, index);
}

patches = [{type: "splice", insert: newVal, index: index, deleteCount: 0}];
dispatched = {
type: how,
patches: patches
};

//!steal-remove-start
if(process.env.NODE_ENV !== 'production') {
dispatched.reasonLog = [ canReflect.getName(this), "added", newVal, "at", index ];
}
//!steal-remove-end
this.dispatch(dispatched, [ newVal, index ]);

} else if (how === 'remove') {
if (itemsDefinition && typeof itemsDefinition.removed === 'function') {
ObservationRecorder.ignore(itemsDefinition.removed).call(this, oldVal, index);
}

patches = [{type: "splice", index: index, deleteCount: oldVal.length}];
dispatched = {
type: how,
patches: patches
};
//!steal-remove-start
if(process.env.NODE_ENV !== 'production') {
dispatched.reasonLog = [ canReflect.getName(this), "remove", oldVal, "at", index ];
}
//!steal-remove-end
this.dispatch(dispatched, [ oldVal, index ]);

} else {
this.dispatch(how, [ newVal, index ]);
}
} else {
this.dispatch({
type: "" + attr,
target: this
}, [ newVal, oldVal ]);
}
}

const MixedInArray = mixinTypeEvents(mixinMapProps(ProxyArray));

class DefineArray extends MixedInArray {
Expand Down Expand Up @@ -169,15 +114,6 @@ class DefineArray extends MixedInArray {

queues.batch.start();
var removed = super.splice.apply(this, args);

if (howMany > 0) {
// tears down bubbling
triggerChange.call(this, "" + index, "remove", undefined, removed);
}
if (args.length > 2) {
triggerChange.call(this, "" + index, "add", added, removed);
}

queues.batch.stop();
return removed;
}
Expand Down
59 changes: 58 additions & 1 deletion src/helpers.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
const canReflect = require("can-reflect");
const mapBindings = require("can-event-queue/map/map");
const ObservationRecorder = require("can-observation-recorder");
const inSetupSymbol = Symbol.for("can.initializing");

const helpers = {
Expand Down Expand Up @@ -28,7 +30,7 @@ const helpers = {
}
//!steal-remove-end

map.dispatch(dispatched, [newVal, current]);
mapBindings.dispatch.call(map, dispatched, [newVal, current]);
}
}
},
Expand All @@ -41,6 +43,61 @@ const helpers = {
(!keyInfo.protoHasKey && !Object.isSealed(meta.target)) || keyInfo.protoHasKey && (typeof targetValue !== "function"))
);
},
triggerChange: function(attr, how, newVal, oldVal) {
var index = +attr;
// `batchTrigger` direct add and remove events...

// Make sure this is not nested and not an expando
if (!isNaN(index)) {
var itemsDefinition = this._define.definitions["#"];
var patches, dispatched;
if (how === 'add' || how === 'set') {
if (itemsDefinition && typeof itemsDefinition.added === 'function') {
ObservationRecorder.ignore(itemsDefinition.added).call(this, newVal, index);
}

patches = [{type: how, insert: newVal, index: index, deleteCount: 0}];
dispatched = {
type: 'splice',
patches: patches
};

//!steal-remove-start
if(process.env.NODE_ENV !== 'production') {
dispatched.reasonLog = [ canReflect.getName(this), "added", newVal, "at", index ];
}
//!steal-remove-end
this.dispatch(dispatched, [ newVal, index ]);
this.dispatch("length", [this.length, this.length - 1]);
this.dispatch({ type: index }, [ newVal, oldVal ]);

} else if (how === 'remove') {
if (itemsDefinition && typeof itemsDefinition.removed === 'function') {
ObservationRecorder.ignore(itemsDefinition.removed).call(this, oldVal, index);
}

patches = [{type: how, index: index, deleteCount: oldVal.length}];
dispatched = {
type: 'splice',
patches: patches
};
//!steal-remove-start
if(process.env.NODE_ENV !== 'production') {
dispatched.reasonLog = [ canReflect.getName(this), "remove", oldVal, "at", index ];
}
//!steal-remove-end
this.dispatch(dispatched, [ oldVal, index ]);

} else {
this.dispatch(how, [ newVal, index ]);
}
} else {
this.dispatch({
type: "" + attr,
target: this
}, [ newVal, oldVal ]);
}
}
};

module.exports = helpers;
102 changes: 22 additions & 80 deletions src/proxy-array.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ const mapBindings = require("can-event-queue/map/map");
const ObservationRecorder = require("can-observation-recorder");
const {
assignNonEnumerable,
eventDispatcher,
shouldRecordObservationOnAllKeysExceptFunctionsOnProto
shouldRecordObservationOnAllKeysExceptFunctionsOnProto,
triggerChange
} = require("./helpers");
const { mixins } = require("can-define-mixin");

Expand Down Expand Up @@ -165,10 +165,6 @@ function setValueAndOnChange(key, value, target, proxy, onChange) {
}
}

function didLengthChangeCauseDeletions(key, value, old) {
return key === "length" && value < old;
}

const proxyHandlers = {
get(target, key, receiver) {
if (isSymbolLike(key)) {
Expand All @@ -184,65 +180,23 @@ const proxyHandlers = {

set(target, key, newValue, receiver) {
let proxy = proxiedObjects.get(target);
let startingLength = target.length;

setValueAndOnChange(key, newValue, target, proxy, function(hadOwn, oldValue) {
// Determine the patches this change should dispatch
let patches = [{
key: key,
type: hadOwn ? "set" : "add",
value: newValue
}];

let numberKey = !isSymbolLike(key) && +key;

// If we are adding an indexed value like `arr[5] =value` ...
if (Number.isInteger(numberKey)) {
// If we set an enumerable property after the length ...
if (!hadOwn && numberKey > startingLength) {
// ... add patches for those values.
patches.push({
index: startingLength,
deleteCount: 0,
insert: target.slice(startingLength),
type: "splice"
});
} else {
// Otherwise, splice the value into the array.
patches.push.apply(patches, mutateMethods.splice(target,
[numberKey, 1, newValue]));
}
}
let numberKey = !isSymbolLike(key) && +key;

// In the case of deleting items by setting the length of the array,
// add patches that splice the items removed.
// (deleting individual items from an array doesn't change the length; it just creates holes)
if (didLengthChangeCauseDeletions(key, newValue, oldValue)) {
patches.push({
index: newValue,
deleteCount: oldValue - newValue,
insert: [],
type: "splice"
});
}
//!steal-remove-start
let reasonLog = [canReflect.getName(proxy)+ " set", key, "to", newValue];
//!steal-remove-end
if (Number.isInteger(numberKey)) {
key = numberKey;
}

let dispatchArgs = {
type: key,
patches: patches,
keyChanged: !hadOwn ? key : undefined
};
setValueAndOnChange(key, newValue, target, proxy, function onChange(hadOwn, oldValue) {

//!steal-remove-start
if(process.env.NODE_ENV !== 'production') {
dispatchArgs.reasonLog = reasonLog;
if (Number.isInteger(key)) {
triggerChange.call(
receiver,
key,
hadOwn ? (newValue ? "set" : "remove") : "add",
newValue,
oldValue
);
}
//!steal-remove-end

//mapBindings.dispatch.call( meta.proxy, dispatchArgs, [value, old]);
eventDispatcher(receiver, key, oldValue, newValue);
});

return true;
Expand All @@ -253,25 +207,13 @@ const proxyHandlers = {

// Fire event handlers if we were able to delete and the value changed.
if (deleteSuccessful && this.preventSideEffects === 0 && old !== undefined) {
//!steal-remove-start
let reasonLog = [canReflect.getName(this.proxy) + " deleted", key];
//!steal-remove-end
// wrapping in process.env.NODE_ENV !== 'production' causes out of scope error
let dispatchArgs = {
type: key,
patches: [{
key: key,
type: "delete"
}],
keyChanged: key
};
//!steal-remove-start
if(process.env.NODE_ENV !== 'production') {
dispatchArgs.reasonLog = reasonLog;
}
//!steal-remove-end

eventDispatcher(this.proxy, dispatchArgs, [undefined, old]);
triggerChange.call(
this.proxy,
key,
"remove",
undefined,
old
);
}

return deleteSuccessful;
Expand Down
Loading

0 comments on commit 676cee4

Please sign in to comment.