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

[MAJOR]: rewrite for v3 #379

Merged
merged 77 commits into from
Nov 27, 2019
Merged
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
77 commits
Select commit Hold shift + click to select a range
a409d5b
Maybe native Proxy?
snewcomer Nov 7, 2019
5ab3b65
Some changes
snewcomer Nov 8, 2019
8e95c93
add evented decorator
snewcomer Nov 8, 2019
d01d6fa
Merge branch 'master' into sn/proxy
snewcomer Nov 11, 2019
9080648
more improvements...still not working
snewcomer Nov 11, 2019
51bbb89
add constructor
snewcomer Nov 11, 2019
ad682d2
IChangesetDef types
snewcomer Nov 11, 2019
9e2f4fa
start separating out files
snewcomer Nov 12, 2019
3d78cc7
start unwinding emberisms
snewcomer Nov 12, 2019
69ef57b
use notifyPropertyChange itself
snewcomer Nov 13, 2019
a1749d1
simplify w/o ember-isms
snewcomer Nov 13, 2019
df3d415
set resulting
snewcomer Nov 14, 2019
f718828
Merge branch 'master' into sn/proxy
snewcomer Nov 14, 2019
c772951
fix setter
snewcomer Nov 15, 2019
7ec77ce
change from getProperty to get to align with Ember APIs
snewcomer Nov 15, 2019
c85a33a
finish tests
snewcomer Nov 16, 2019
c213876
remove unused files
snewcomer Nov 16, 2019
efab568
move around and remove uneeded files
snewcomer Nov 16, 2019
a8df21b
overridable set deep
snewcomer Nov 16, 2019
cd4490f
bump deps
snewcomer Nov 16, 2019
786f95a
remove uneeded property
snewcomer Nov 16, 2019
0d93f8e
minor fixes
snewcomer Nov 16, 2019
1d1f1d7
lets start working with single key level maps
snewcomer Nov 17, 2019
75a8cf8
get most working
snewcomer Nov 18, 2019
79dfb28
remove unused inflate
snewcomer Nov 18, 2019
88131ee
merge deep on the content
snewcomer Nov 18, 2019
ca62792
cleanup on deep merge utility
snewcomer Nov 18, 2019
40ff0fd
no more set-nested
snewcomer Nov 18, 2019
1ae403a
moar comments
snewcomer Nov 18, 2019
d30d7e5
some improvements to tests and imports
snewcomer Nov 19, 2019
2cfd7c1
some improvements to merge-deep
snewcomer Nov 19, 2019
82af80b
update testing Ci
snewcomer Nov 19, 2019
d9c6a62
fix merge deep by looking for value
snewcomer Nov 22, 2019
004d77c
address keyValues map and add documentation
snewcomer Nov 22, 2019
5c509f4
address notifier feedback
snewcomer Nov 22, 2019
ae1afb7
fix merge-deep to be more recursive like
snewcomer Nov 23, 2019
42a22d2
addError overload
snewcomer Nov 23, 2019
e5ed01b
use Record for get-deep
snewcomer Nov 23, 2019
d18b6fb
add type guard
snewcomer Nov 23, 2019
ffc538a
return early if value on target
snewcomer Nov 23, 2019
9be116d
move utils to utils folder
snewcomer Nov 23, 2019
8bc34b6
add some tests around normalize obj and pure-assign
snewcomer Nov 23, 2019
ca58df3
add get deep tests
snewcomer Nov 23, 2019
75b2670
add set deep test
snewcomer Nov 23, 2019
c36b81f
fix travis
snewcomer Nov 23, 2019
20b3438
ts ignore
snewcomer Nov 23, 2019
e4750ed
ember tsc 2.0
snewcomer Nov 23, 2019
e137dda
add babel to deps
snewcomer Nov 23, 2019
6dffd76
add back experimentalDecorators
snewcomer Nov 23, 2019
94f8c75
type check spread
snewcomer Nov 23, 2019
c5bf3cd
bump dep
snewcomer Nov 23, 2019
b4f58a8
Update more deps
snewcomer Nov 23, 2019
afee3ff
add glimmer component
snewcomer Nov 23, 2019
b91aaff
strict property initialization
snewcomer Nov 23, 2019
d0f2bde
update yarn file
snewcomer Nov 23, 2019
49785c1
add glimmer/tracking and remove glimmer/component
snewcomer Nov 23, 2019
a4499dc
skipLibCheck
snewcomer Nov 24, 2019
a8feb0e
need one?
snewcomer Nov 24, 2019
a5085d2
3.13
snewcomer Nov 24, 2019
768026a
qunit codemod
snewcomer Nov 24, 2019
0f0d6cf
isobject into own property @makepanic
snewcomer Nov 24, 2019
b4fdcd4
cleanup
snewcomer Nov 24, 2019
9c08ce2
format
snewcomer Nov 24, 2019
4eac8fd
add back assertions
snewcomer Nov 24, 2019
9215679
add some mergeDeep tests
snewcomer Nov 24, 2019
f373925
remove validate nested obj utility
snewcomer Nov 24, 2019
bc55557
use Change as common way of accessing
snewcomer Nov 24, 2019
afdebd5
remove ember-deep-set
snewcomer Nov 24, 2019
6224202
Update README with v3.0 install
snewcomer Nov 24, 2019
4301bb4
notifier cleanup
snewcomer Nov 24, 2019
2f8267d
clarifications on using Ember.set
snewcomer Nov 24, 2019
f4ed5ac
clarify #set with nested keys
snewcomer Nov 24, 2019
9760ff1
add another helpful dev assert
snewcomer Nov 25, 2019
8cf6f2d
remove deepSet type
snewcomer Nov 25, 2019
54c6ba8
add tests for native setter
snewcomer Nov 26, 2019
ab62972
ensure can call nested with error and change
snewcomer Nov 27, 2019
472576f
add contributors
snewcomer Nov 27, 2019
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
14 changes: 9 additions & 5 deletions .eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
module.exports = {
root: true,
parser: 'babel-eslint',
parserOptions: {
ecmaVersion: 2017,
sourceType: 'module'
ecmaVersion: 2018,
sourceType: 'module',
ecmaFeatures: {
legacyDecorators: true
}
},
plugins: [
'ember'
Expand All @@ -15,13 +19,14 @@ module.exports = {
browser: true
},
rules: {
'ember/avoid-leaking-state-in-ember-objects': 0
'ember/no-jquery': 'error'
},
overrides: [
// node files
{
files: [
'.eslintrc.js',
'.template-lintrc.js',
'ember-cli-build.js',
'index.js',
'testem.js',
Expand All @@ -36,8 +41,7 @@ module.exports = {
'tests/dummy/app/**'
],
parserOptions: {
sourceType: 'script',
ecmaVersion: 2015
sourceType: 'script'
},
env: {
browser: false,
Expand Down
3 changes: 2 additions & 1 deletion .npmignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@
# misc
/.bowerrc
/.editorconfig
/.ember-cli
/.ember-cli.js
/.env*
/.eslintignore
/.eslintrc.js
/.git/
/.gitignore
/.template-lintrc.js
/.travis.yml
Expand Down
5 changes: 2 additions & 3 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ jobs:
install:
- yarn install --non-interactive
script:
- yarn lint
- yarn lint:hbs
- yarn lint:js
- yarn test

- name: "Floating Dependencies"
Expand All @@ -48,8 +49,6 @@ jobs:
# we recommend new addons test the current and previous LTS
# as well as latest stable release (bonus points to beta/canary)
- stage: "Additional Tests"
env: EMBER_TRY_SCENARIO=ember-lts-2.18
- env: EMBER_TRY_SCENARIO=ember-lts-3.4
- env: EMBER_TRY_SCENARIO=ember-release
- env: EMBER_TRY_SCENARIO=ember-beta
- env: EMBER_TRY_SCENARIO=ember-canary
Expand Down
15 changes: 15 additions & 0 deletions addon/-private/evented.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import Notifier from './notifier';

export function notifierForEvent(object: any, eventName: string) {
if (object._eventedNotifiers === undefined) {
object._eventedNotifiers = {};
}

let notifier = object._eventedNotifiers[eventName];

if (!notifier) {
notifier = object._eventedNotifiers[eventName] = new Notifier();
}

return notifier;
}
19 changes: 19 additions & 0 deletions addon/-private/get-deep.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
export default function getDeep<T extends object>(root: T, path: string | string[]): any {
Copy link

Choose a reason for hiding this comment

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

Why are you using a generic here?

let obj: any = root;

if (path.indexOf('.') === -1) {
return obj[path as string];
}
let parts = typeof path === 'string' ? path.split('.') : path;

for (let i = 0; i < parts.length; i++) {
if (obj === undefined || obj === null) {
return undefined;
}

// next iteration has next level
obj = obj[parts[i]];
}

return obj;
}
29 changes: 29 additions & 0 deletions addon/-private/key-values.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import isObject from '../utils/is-object';

// 'foo.bar.zaz'
let keysUpToValue: any[] = [];
Copy link

Choose a reason for hiding this comment

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

If this is used as an accumulator for recursion, this should be an argument to the function. Having it persisted across function calls can be error prone. If you don't want to expose the accumulator outside this module, you can use this common pattern:

const myRecursiveFunctionWithAccumulator = (a, acc) => {
  /* do whatever recursive stuff you need to do */
};
export const myFunction = (a) => myRecursiveFunctionWithAccumulator(a, []);


/**
* traverse through target and return leaf nodes with `value` property
*
* @method keyValues
* @param target
*/
export default function keyValues<T extends { [key: string]: any}>(obj: T): object[] {
Copy link

Choose a reason for hiding this comment

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

{ [key: string]: any } is the same a Record<string, any>

let map = [];
for (let key in obj) {
keysUpToValue.push(key);

if (obj[key] && isObject(obj[key])) {
if (Object.prototype.hasOwnProperty.apply(obj[key], ['value'])) {
Copy link

Choose a reason for hiding this comment

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

If you use call instead of apply you don't need to create an array with the 'value' string.

map.push({ key: keysUpToValue.join('.'), value: obj[key].value });
// stop collecting keys
keysUpToValue = [];
} else if (key !== 'value') {
map.push(...keyValues(obj[key]));
}
}
}

return map;
}
132 changes: 132 additions & 0 deletions addon/-private/merge-deep.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
interface Options {
safeGet: any
safeSet: any
}

function isMergeableObject(value: any): Boolean {
return isNonNullObject(value) && !isSpecial(value);
}

function isNonNullObject(value: any): Boolean {
return !!value && typeof value === 'object';
}

function isSpecial(value: any): Boolean {
let stringValue = Object.prototype.toString.call(value);

return stringValue === '[object RegExp]' || stringValue === '[object Date]';
}

function getEnumerableOwnPropertySymbols(target: any): any {
return Object.getOwnPropertySymbols
? Object.getOwnPropertySymbols(target).filter(symbol => {
return target.propertyIsEnumerable(symbol)
})
: [];
}

function getKeys(target: any) {
return Object.keys(target).concat(getEnumerableOwnPropertySymbols(target))
}

function propertyIsOnObject(object: any, property: any) {
try {
return property in object;
} catch(_) {
return false;
}
}

// Protects from prototype poisoning and unexpected merging up the prototype chain.
function propertyIsUnsafe(target: any, key: string): Boolean {
return propertyIsOnObject(target, key) // Properties are safe to merge if they don't exist in the target yet,
&& !(Object.hasOwnProperty.call(target, key) // unsafe if they exist up the prototype chain,
&& Object.propertyIsEnumerable.call(target, key)); // and also unsafe if they're nonenumerable.
Copy link

Choose a reason for hiding this comment

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

Correct indentation here. You might want to add prettier to keep consistency between tabs / spaces.

}

let kv: { [k: string]: any } = {};
let possibleKeys: string[] = [];

/**
* DFS - traverse depth first until find object with `value`. Then go back up tree and try on next key
* need to exhaust all possible avenues.
*
* @method buildPathToValue
*/
function buildPathToValue(source: any, options: Options): void {
Object.keys(source).forEach((key: string): void => {
let possible = source[key];
if (possible && possible.hasOwnProperty('value')) {
possibleKeys.push(key);
kv[possibleKeys.join('.')] = possible.value;
possibleKeys = [];
return;
}

if (typeof possible === 'object') {
possibleKeys.push(key);
buildPathToValue(possible, options);
} else {
possibleKeys = [];
}
});
}

/**
* `source` will always have a leaf key `value` with the proeprty we want to set
Copy link

Choose a reason for hiding this comment

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

s/proeprty/property/

* @method mergeObject
*/
function mergeObject(target: any, source: any, options: Options) {
getKeys(source).forEach(key => {
// proto poisoning. So can set by nested key path 'person.name'
if (propertyIsUnsafe(target, key)) {
if (options.safeSet) {
buildPathToValue(source, options);
if (Object.keys(kv).length > 0) {
// we found some keys!
for (key in kv) {
const val = kv[key];
options.safeSet(target, key, val);
}
}

kv = {};
}

return;
}

// else safe key on object
if (propertyIsOnObject(target, key) && isMergeableObject(source[key]) && !source[key].hasOwnProperty('value')) {
target[key] = mergeDeep(options.safeGet(target, key), options.safeGet(source, key), options);
} else {
return target[key] = source[key].value;
}
});

return target;
}

/**
* goal is to mutate target with source's properties, ensuring we dont encounter
* pitfalls of { ..., ... } spread syntax overwriting keys on objects that we merged
*
* @method mergeDeep
* @param target
* @param source
*/
export default function mergeDeep(target: any, source: any, options: Options = { safeGet: undefined, safeSet: undefined }): object | [any] {
options['safeGet'] = options.safeGet || function(obj: any, key: string): any { return obj[key] };
Copy link

Choose a reason for hiding this comment

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

Why the bracket?

options['safeSet'] = options.safeSet;
let sourceIsArray = Array.isArray(source);
let targetIsArray = Array.isArray(target);
let sourceAndTargetTypesMatch = sourceIsArray === targetIsArray;

if (!sourceAndTargetTypesMatch) {
return source;
} else if (sourceIsArray) {
return source;
} else {
return mergeObject(target, source, options);
}
}
29 changes: 29 additions & 0 deletions addon/-private/normalize-object.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import isObject from '../utils/is-object';

/**
* traverse through target and unset `value` from leaf key
* Shallow copy here is fine because we are swapping out the leaf nested object
* rather than mutating a property in something with reference
*
* @method normalizeObject
* @param target
*/
export default function normalizeObject<T extends { [key: string]: any}>(target: T): T {
Copy link

Choose a reason for hiding this comment

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

Why the generic?

let obj = { ...target };

for (let key in obj) {
if (key === 'value') {
Copy link

Choose a reason for hiding this comment

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

Can we get this check to be the first thing to do?

if ('value' in target) { // Or hasOwnProperty
  return target.value;
}

That way we prevent creating a new object and traversing any property.

return obj[key];
}

if (obj[key] && isObject(obj[key])) {
if (Object.prototype.hasOwnProperty.apply(obj[key], ['value'])) {
obj[key] = obj[key].value;
} else {
obj[key] = normalizeObject(obj[key]);
}
}
}

return obj;
}
30 changes: 30 additions & 0 deletions addon/-private/notifier.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// this statefull class holds and notifies
import { INotifier } from '../types/evented';

export default class Notifier implements INotifier {
Copy link

Choose a reason for hiding this comment

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

In general, types like Function, String, Number, etc. should not be used.

listeners: Function[]

constructor() {
this.listeners = [];
}

addListener(callback: Function) {
this.listeners.push(callback);
return () => this.removeListener(callback);
}

removeListener(callback: Function) {
this.listeners;

for (let i = 0; i < this.listeners.length; i++) {
if (this.listeners[i] === callback) {
this.listeners.splice(i, 1);
return;
}
}
}

trigger(...args: any[]) {
this.listeners.slice(0).forEach(callback => callback(...args));
Copy link

Choose a reason for hiding this comment

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

Why do you need the slice?

}
}
Loading