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

Use async observer when target app is Ember 3.13+ #777

Merged
merged 8 commits into from
Oct 30, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
18 changes: 18 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,23 @@ module.exports = {
'ember/avoid-leaking-state-in-ember-objects': 'off',

'ember-best-practices/require-dependent-keys': 'off',

'no-restricted-imports': [
'error',
{
paths: [
{
name: '@ember/object',
importNames: ['observer'],
message: 'For compatibility, import observers from -private/utils/observer',
bantic marked this conversation as resolved.
Show resolved Hide resolved
},
{
name: '@ember/object/observers',
importNames: ['addObserver', 'removeObserver'],
message: 'For compatibility, import observers from -private/utils/observer',
},
],
},
],
},
};
2 changes: 1 addition & 1 deletion addon/-private/collapse-tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import EmberArray, { A as emberA, isArray } from '@ember/array';
import { assert, warn } from '@ember/debug';

import { computed } from '@ember/object';
import { addObserver } from '@ember/object/observers';
import { addObserver } from './utils/observer';

import { objectAt } from './utils/array';
import { notifyPropertyChange } from './utils/ember';
Expand Down
2 changes: 1 addition & 1 deletion addon/-private/column-tree.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint-disable getter-return */
import EmberObject, { get, set } from '@ember/object';
import { addObserver, removeObserver } from '@ember/object/observers';
import { addObserver, removeObserver } from './utils/observer';
import { A as emberA } from '@ember/array';
import { DEBUG } from '@glimmer/env';

Expand Down
3 changes: 2 additions & 1 deletion addon/-private/utils/computed.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import EmberObject, { defineProperty, computed, observer } from '@ember/object';
import EmberObject, { defineProperty, computed } from '@ember/object';
import { observer } from './observer';
import { alias } from '@ember/object/computed';

const PROPERTIES = new WeakMap();
Expand Down
50 changes: 50 additions & 0 deletions addon/-private/utils/observer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { gte } from 'ember-compatibility-helpers';
import { assert } from '@ember/debug';

// eslint-disable-next-line no-restricted-imports
import { observer as eObserver } from '@ember/object';

// eslint-disable-next-line no-restricted-imports
import {
addObserver as eAddObserver,
bantic marked this conversation as resolved.
Show resolved Hide resolved
removeObserver as eRemoveObserver,
} from '@ember/object/observers';

const USE_ASYNC_OBSERVERS = gte('3.13.0');

function asyncObserver(...args) {
let fn = args.pop();
let dependentKeys = args;
let sync = false;

return eObserver({ dependentKeys, fn, sync });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

}

function asyncAddObserver(...args) {
let obj, path, target, method;
let sync = false;
obj = args[0];
path = args[1];
assert(
`Expected 3 or 4 args for addObserver, got ${args.length}`,
args.length === 3 || args.length === 4
);
if (args.length === 3) {
target = null;
method = args[2];
} else if (args.length === 4) {
target = args[2];
method = args[3];
}

return eAddObserver(obj, target, path, method, sync);
}

function asyncRemoveObserver(key, target, method) {
let sync = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

could also just pass false to eRemoveObserver but it is definitely more declarative

return eRemoveObserver(key, target, method, sync);
}

export const observer = USE_ASYNC_OBSERVERS ? asyncObserver : eObserver;
export const addObserver = USE_ASYNC_OBSERVERS ? asyncAddObserver : eAddObserver;
export const removeObserver = USE_ASYNC_OBSERVERS ? asyncRemoveObserver : eRemoveObserver;
3 changes: 2 additions & 1 deletion addon/components/-private/base-table-cell.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import Component from '@ember/component';
import { equal } from '@ember/object/computed';
import { observer, computed } from '@ember/object';
import { observer } from '../../-private/utils/observer';
import { scheduleOnce } from '@ember/runloop';
import { computed } from '@ember/object';

export default Component.extend({
// Provided by subclasses
Expand Down
3 changes: 2 additions & 1 deletion addon/components/ember-tbody/component.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import Component from '@ember/component';

import { computed, observer } from '@ember/object';
import { computed } from '@ember/object';
import { observer } from '../../-private/utils/observer';
import { bool, readOnly, or } from '@ember/object/computed';

import { SUPPORTS_INVERSE_BLOCK } from 'ember-compatibility-helpers';
Expand Down
25 changes: 13 additions & 12 deletions addon/components/ember-thead/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import Component from '@ember/component';
import { bind } from '@ember/runloop';
import { A as emberA } from '@ember/array';
import defaultTo from '../../-private/utils/default-to';
import { addObserver } from '../../-private/utils/observer';
import { computed } from '@ember/object';
import { notEmpty, or, readOnly } from '@ember/object/computed';

Expand Down Expand Up @@ -213,20 +214,20 @@ export default Component.extend({
this._updateApi();
this._updateColumnTree();

this.addObserver('sorts', this._updateApi);
this.addObserver('sortFunction', this._updateApi);
this.addObserver('reorderFunction', this._updateApi);
addObserver(this, 'sorts', this._updateApi);
addObserver(this, 'sortFunction', this._updateApi);
addObserver(this, 'reorderFunction', this._updateApi);

this.addObserver('sorts', this._updateColumnTree);
this.addObserver('columns.[]', this._onColumnsChange);
this.addObserver('fillMode', this._updateColumnTree);
this.addObserver('fillColumnIndex', this._updateColumnTree);
this.addObserver('resizeMode', this._updateColumnTree);
this.addObserver('widthConstraint', this._updateColumnTree);
addObserver(this, 'sorts', this._updateColumnTree);
addObserver(this, 'columns.[]', this._onColumnsChange);
addObserver(this, 'fillMode', this._updateColumnTree);
addObserver(this, 'fillColumnIndex', this._updateColumnTree);
addObserver(this, 'resizeMode', this._updateColumnTree);
addObserver(this, 'widthConstraint', this._updateColumnTree);

this.addObserver('enableSort', this._updateColumnTree);
this.addObserver('enableResize', this._updateColumnTree);
this.addObserver('enableReorder', this._updateColumnTree);
addObserver(this, 'enableSort', this._updateColumnTree);
addObserver(this, 'enableResize', this._updateColumnTree);
addObserver(this, 'enableReorder', this._updateColumnTree);
},

_updateApi() {
Expand Down
9 changes: 0 additions & 9 deletions config/ember-try.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,6 @@ module.exports = function() {
},
{
name: 'ember-release',
env: {
SKIP_REORDERING_TESTS: true,
},
npm: {
devDependencies: {
'ember-source': urls[0],
Expand All @@ -179,9 +176,6 @@ module.exports = function() {
},
{
name: 'ember-beta',
env: {
SKIP_REORDERING_TESTS: true,
},
npm: {
devDependencies: {
'ember-source': urls[1],
Expand All @@ -191,9 +185,6 @@ module.exports = function() {
},
{
name: 'ember-canary',
env: {
SKIP_REORDERING_TESTS: true,
},
npm: {
devDependencies: {
'ember-source': urls[2],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import Controller from '@ember/controller';
import { computed } from '@ember/object';
import { generateRows } from '../../../../../utils/generators';
import { A } from '@ember/array';

export default Controller.extend({
rows: computed(function() {
Expand Down
1 change: 0 additions & 1 deletion tests/dummy/config/environment.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ module.exports = function(environment) {
fastboot: {
hostWhitelist: [/^localhost:\d+$/],
},
skipReorderingTests: process.env.SKIP_REORDERING_TESTS,
EmberENV: {
FEATURES: {
// Here you can enable experimental features on an ember canary build
Expand Down
7 changes: 0 additions & 7 deletions tests/integration/components/headers/reorder-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,6 @@ async function reorderToRightEdge(column, edgeOffset = 0) {
}

module('Integration | headers | reorder', function() {
if (config.skipReorderingTests) {
skip('Skipping column reordering tests, see https://github.com/Addepar/ember-table/issues/775', async function(assert) {
assert.ok(true, 'Skipping column reordering tests');
});
return;
}

componentModule('reordering', function() {
test('standard columns', async function(assert) {
await generateTable(this);
Expand Down
3 changes: 2 additions & 1 deletion tests/integration/components/selection-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { A as emberA } from '@ember/array';
import { run } from '@ember/runloop';
import { scrollTo } from 'ember-native-dom-helpers';
import { registerTestWarnHandler } from '../../helpers/warn-handlers';
import wait from 'ember-test-helpers/wait';

let table = new TablePage({
validateSelected(...selectedIndexes) {
Expand Down Expand Up @@ -585,7 +586,7 @@ module('Integration | selection', () => {
let rows = generateRows(1, 1);
await generateTable(this, { rows });

this.set('selection', [...rows, { fakeRow: true }]);
Ember.run(() => this.set('selection', [...rows, { fakeRow: true }]));
assert.ok(true, 'after setting bad selection, no error');
assert.ok(table.validateSelected(0), 'First row is selected');

Expand Down
59 changes: 10 additions & 49 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@
resolved "https://registry.npmjs.org/@babel/parser/-/parser-7.6.0.tgz#3e05d0647432a8326cb28d0de03895ae5a57f39b"
integrity sha512-+o2q111WEx4srBs7L9eJmcwi655eD8sXniLqMB93TBK9GrNzGrxDWSjiqz2hLU0Ha8MTXFIP0yd9fNdP+m43ZQ==

"@babel/parser@^7.3.4", "@babel/parser@^7.4.5", "@babel/parser@^7.6.3", "@babel/parser@^7.6.4":
"@babel/parser@^7.3.4", "@babel/parser@^7.6.3", "@babel/parser@^7.6.4":
version "7.6.4"
resolved "https://registry.npmjs.org/@babel/parser/-/parser-7.6.4.tgz#cb9b36a7482110282d5cb6dd424ec9262b473d81"
integrity sha512-D8RHPW5qd0Vbyo3qb+YjO5nvUVRTXFLQ/FsDxJU2Nqz4uB5EnUN0ZQSEYpvTIbRuttig1XbHWU5oMeQwQSAA+A==
Expand Down Expand Up @@ -475,7 +475,7 @@
dependencies:
"@babel/helper-plugin-utils" "^7.0.0"

"@babel/plugin-transform-block-scoping@^7.4.4", "@babel/plugin-transform-block-scoping@^7.5.5":
"@babel/plugin-transform-block-scoping@^7.5.5":
version "7.6.3"
resolved "https://registry.npmjs.org/@babel/plugin-transform-block-scoping/-/plugin-transform-block-scoping-7.6.3.tgz#6e854e51fbbaa84351b15d4ddafe342f3a5d542a"
integrity sha512-7hvrg75dubcO3ZI2rjYTzUrEuh1E9IyDEhhB6qfcooxhDA33xx2MasuLVgdxzcP6R/lipAC6n9ub9maNW6RKdw==
Expand Down Expand Up @@ -622,13 +622,6 @@
dependencies:
"@babel/helper-plugin-utils" "^7.0.0"

"@babel/plugin-transform-object-assign@^7.2.0":
version "7.2.0"
resolved "https://registry.npmjs.org/@babel/plugin-transform-object-assign/-/plugin-transform-object-assign-7.2.0.tgz#6fdeea42be17040f119e38e23ea0f49f31968bde"
integrity sha512-nmE55cZBPFgUktbF2OuoZgPRadfxosLOpSgzEPYotKSls9J4pEPcembi8r78RU37Rph6UApCpNmsQA4QMWK9Ng==
dependencies:
"@babel/helper-plugin-utils" "^7.0.0"

"@babel/plugin-transform-object-super@^7.5.5":
version "7.5.5"
resolved "https://registry.npmjs.org/@babel/plugin-transform-object-super/-/plugin-transform-object-super-7.5.5.tgz#c70021df834073c65eb613b8679cc4a381d1a9f9"
Expand Down Expand Up @@ -826,7 +819,7 @@
globals "^11.1.0"
lodash "^4.17.13"

"@babel/traverse@^7.1.6", "@babel/traverse@^7.2.4", "@babel/traverse@^7.3.4", "@babel/traverse@^7.4.5", "@babel/traverse@^7.6.2", "@babel/traverse@^7.6.3":
"@babel/traverse@^7.1.6", "@babel/traverse@^7.2.4", "@babel/traverse@^7.3.4", "@babel/traverse@^7.6.2", "@babel/traverse@^7.6.3":
version "7.6.3"
resolved "https://registry.npmjs.org/@babel/traverse/-/traverse-7.6.3.tgz#66d7dba146b086703c0fb10dd588b7364cec47f9"
integrity sha512-unn7P4LGsijIxaAJo/wpoU11zN+2IaClkQAxcJWBNCMS6cmVh802IyLHNkAjQ0iYnRS3nnxk5O3fuXW28IMxTw==
Expand Down Expand Up @@ -1957,11 +1950,6 @@ assign-symbols@^1.0.0:
resolved "https://registry.npmjs.org/assign-symbols/-/assign-symbols-1.0.0.tgz#59667f41fadd4f20ccbc2bb96b8d4f7f78ec0367"
integrity sha1-WWZ/QfrdTyDMvCu5a41Pf3jsA2c=

ast-types@0.13.2:
version "0.13.2"
resolved "https://registry.npmjs.org/ast-types/-/ast-types-0.13.2.tgz#df39b677a911a83f3a049644fb74fdded23cea48"
integrity sha512-uWMHxJxtfj/1oZClOxDEV1sQ1HCDkA4MG8Gr69KKeBjEVH0R84WlejZ0y2DcwyBlpAEMltmVYkVgqfLFb2oyiA==

ast-types@0.9.6:
version "0.9.6"
resolved "https://registry.npmjs.org/ast-types/-/ast-types-0.9.6.tgz#102c9e9e9005d3e7e3829bf0c4fa24ee862ee9b9"
Expand Down Expand Up @@ -2307,7 +2295,7 @@ babel-plugin-debug-macros@^0.2.0, babel-plugin-debug-macros@^0.2.0-beta.6:
dependencies:
semver "^5.3.0"

babel-plugin-debug-macros@^0.3.0, babel-plugin-debug-macros@^0.3.2, babel-plugin-debug-macros@^0.3.3:
babel-plugin-debug-macros@^0.3.0, babel-plugin-debug-macros@^0.3.2:
version "0.3.3"
resolved "https://registry.npmjs.org/babel-plugin-debug-macros/-/babel-plugin-debug-macros-0.3.3.tgz#29c3449d663f61c7385f5b8c72d8015b069a5cb7"
integrity sha512-E+NI8TKpxJDBbVkdWkwHrKgJi696mnRL8XYrOPYw82veNHPDORM9WIQifl6TpIo8PNy2tU2skPqbfkmHXrHKQA==
Expand Down Expand Up @@ -6202,15 +6190,6 @@ ember-router-generator@^1.2.3:
dependencies:
recast "^0.11.3"

ember-router-generator@^2.0.0:
version "2.0.0"
resolved "https://registry.npmjs.org/ember-router-generator/-/ember-router-generator-2.0.0.tgz#d04abfed4ba8b42d166477bbce47fccc672dbde0"
integrity sha512-89oVHVJwmLDvGvAUWgS87KpBoRhy3aZ6U0Ql6HOmU4TrPkyaa8pM0W81wj9cIwjYprcQtN9EwzZMHnq46+oUyw==
dependencies:
"@babel/parser" "^7.4.5"
"@babel/traverse" "^7.4.5"
recast "^0.18.1"

ember-router-scroll@^1.0.0:
version "1.3.3"
resolved "https://registry.npmjs.org/ember-router-scroll/-/ember-router-scroll-1.3.3.tgz#411991a671bd970497f5ce757baa627e850ae6e0"
Expand All @@ -6234,18 +6213,11 @@ ember-source-channel-url@^1.0.1:
dependencies:
got "^8.0.1"

ember-source@~3.13:
version "3.13.3"
resolved "https://registry.npmjs.org/ember-source/-/ember-source-3.13.3.tgz#65794f79dec0512d833332575f13c77f9ccae2a1"
integrity sha512-aDmzAwpCa4H6ozd+RbsQs9/Pfo4wbnDVe9eb2D05PH9W6zRpiUa+pTluJsUFDfbi+jYGPQnjty2U/UQYBayFvg==
ember-source@~3.12:
version "3.12.0"
resolved "https://registry.npmjs.org/ember-source/-/ember-source-3.12.0.tgz#92f72894836d4497e704901c1d061c61b066bddf"
integrity sha512-4iA2BgYmNLWysifLyt2LCQgU9ux/NiTR/MT7KTt9HUyTDJyivcdyKNtfrUQst/1InUvn+MxuQ0ZsbQICJkX6yA==
dependencies:
"@babel/helper-module-imports" "^7.0.0"
"@babel/plugin-transform-block-scoping" "^7.4.4"
"@babel/plugin-transform-object-assign" "^7.2.0"
"@ember/edition-utils" "^1.1.1"
babel-plugin-debug-macros "^0.3.3"
babel-plugin-filter-imports "^3.0.0"
broccoli-concat "^3.7.3"
broccoli-funnel "^2.0.2"
broccoli-merge-trees "^3.0.2"
chalk "^2.4.2"
Expand All @@ -6256,11 +6228,10 @@ ember-source@~3.13:
ember-cli-path-utils "^1.0.0"
ember-cli-string-utils "^1.1.0"
ember-cli-version-checker "^3.1.3"
ember-router-generator "^2.0.0"
ember-router-generator "^1.2.3"
inflection "^1.12.0"
jquery "^3.4.1"
resolve "^1.11.1"
silent-error "^1.1.1"

ember-svg-jar@^1.2.2:
version "1.2.2"
Expand Down Expand Up @@ -6917,7 +6888,7 @@ esprima@^3.1.3, esprima@~3.1.0:
resolved "https://registry.npmjs.org/esprima/-/esprima-3.1.3.tgz#fdca51cee6133895e3c88d535ce49dbff62a4633"
integrity sha1-/cpRzuYTOJXjyI1TXOSdv/YqRjM=

esprima@^4.0.0, esprima@~4.0.0:
esprima@^4.0.0:
version "4.0.1"
resolved "https://registry.npmjs.org/esprima/-/esprima-4.0.1.tgz#13b04cdb3e6c5d19df91ab6987a8695619b0aa71"
integrity sha512-eGuFFw7Upda+g4p+QHvnW0RyTX/SVeJBDM/gCtMARO0cLuT2HcEKnTPvhjV6aGeqrCB/sbNop0Kszm0jsaWU4A==
Expand Down Expand Up @@ -12089,16 +12060,6 @@ recast@^0.11.3:
private "~0.1.5"
source-map "~0.5.0"

recast@^0.18.1:
version "0.18.5"
resolved "https://registry.npmjs.org/recast/-/recast-0.18.5.tgz#9d5adbc07983a3c8145f3034812374a493e0fe4d"
integrity sha512-sD1WJrpLQAkXGyQZyGzTM75WJvyAd98II5CHdK3IYbt/cZlU0UzCRVU11nUFNXX9fBVEt4E9ajkMjBlUlG+Oog==
dependencies:
ast-types "0.13.2"
esprima "~4.0.0"
private "^0.1.8"
source-map "~0.6.1"

rechoir@^0.6.2:
version "0.6.2"
resolved "https://registry.npmjs.org/rechoir/-/rechoir-0.6.2.tgz#85204b54dba82d5742e28c96756ef43af50e3384"
Expand Down