From 5611d366b216fa61ff27068a9234a9dfd82f165c Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Fri, 30 Jun 2023 16:56:54 -0700 Subject: [PATCH 1/3] dx: better debugging for backtracking errors --- packages/model/src/-private/record-state.ts | 10 ++- packages/store/src/-private/cache-handler.ts | 2 + .../record-arrays/identifier-array.ts | 14 +++- packages/tracking/addon-main.js | 76 ++++++++++++++++++- packages/tracking/babel.config.js | 13 ++++ packages/tracking/babel.config.json | 8 -- packages/tracking/package.json | 10 ++- packages/tracking/rollup.config.mjs | 2 +- packages/tracking/src/-private.ts | 64 +++++++++++++++- pnpm-lock.yaml | 14 ++++ .../meaningful-backtracking-errors-test.js | 51 +++++++++++++ 11 files changed, 247 insertions(+), 17 deletions(-) create mode 100644 packages/tracking/babel.config.js delete mode 100644 packages/tracking/babel.config.json create mode 100644 tests/main/tests/end-user-dx/meaningful-backtracking-errors-test.js diff --git a/packages/model/src/-private/record-state.ts b/packages/model/src/-private/record-state.ts index 904b65f0bd0..3b1720b6830 100644 --- a/packages/model/src/-private/record-state.ts +++ b/packages/model/src/-private/record-state.ts @@ -38,8 +38,15 @@ class Tag { declare isDirty: boolean; declare value: any; declare t: boolean; + declare _debug_base: string; + declare _debug_prop: string; constructor() { + if (DEBUG) { + const [base, prop] = arguments as unknown as [string, string]; + this._debug_base = base; + this._debug_prop = prop; + } this.rev = 1; this.isDirty = true; this.value = undefined; @@ -68,7 +75,8 @@ function getTag(record, key) { tags = Object.create(null); Tags.set(record, tags); } - return (tags[key] = tags[key] || new Tag()); + // @ts-expect-error + return (tags[key] = tags[key] || (DEBUG ? new Tag(record.constructor.modelName, key) : new Tag())); } export function peekTag(record, key) { diff --git a/packages/store/src/-private/cache-handler.ts b/packages/store/src/-private/cache-handler.ts index 8700f5ffa08..5fa3dc3f549 100644 --- a/packages/store/src/-private/cache-handler.ts +++ b/packages/store/src/-private/cache-handler.ts @@ -79,6 +79,7 @@ function maybeUpdateUiObjects( return document as T; } const data = recordArrayManager.createArray({ + type: request.url, identifiers: document.data, doc: document as CollectionResourceDataDocument, query: request, @@ -95,6 +96,7 @@ function maybeUpdateUiObjects( if (!managed) { managed = recordArrayManager.createArray({ + type: identifier.lid, identifiers: document.data, doc: document as CollectionResourceDataDocument, }); diff --git a/packages/store/src/-private/record-arrays/identifier-array.ts b/packages/store/src/-private/record-arrays/identifier-array.ts index b95e88a111d..091c82e1f20 100644 --- a/packages/store/src/-private/record-arrays/identifier-array.ts +++ b/packages/store/src/-private/record-arrays/identifier-array.ts @@ -74,7 +74,7 @@ export const NOTIFY = Symbol('#notify'); const IS_COLLECTION = Symbol.for('Collection'); export function notifyArray(arr: IdentifierArray) { - arr[IDENTIFIER_ARRAY_TAG].ref = null; + addToTransaction(arr[IDENTIFIER_ARRAY_TAG]); if (DEPRECATE_COMPUTED_CHAINS) { // eslint-disable-next-line @@ -101,8 +101,16 @@ class Tag { * whether this was part of a transaction when last mutated */ declare t: boolean; + declare _debug_base: string; + declare _debug_prop: string; constructor() { + if (DEBUG) { + const [arr, prop] = arguments as unknown as [IdentifierArray, string]; + + this._debug_base = arr.constructor.name + ':' + String(arr.modelName); + this._debug_prop = prop; + } this.shouldReset = false; this.t = false; } @@ -207,7 +215,7 @@ class IdentifierArray { _updatingPromise: PromiseArray | Promise | null = null; [IS_COLLECTION] = true; - [IDENTIFIER_ARRAY_TAG] = new Tag(); + declare [IDENTIFIER_ARRAY_TAG]: Tag; [SOURCE]: StableRecordIdentifier[]; [NOTIFY]() { notifyArray(this); @@ -268,6 +276,8 @@ class IdentifierArray { this.store = options.store; this._manager = options.manager; this[SOURCE] = options.identifiers; + // @ts-expect-error + this[IDENTIFIER_ARRAY_TAG] = DEBUG ? new Tag(this, 'length') : new Tag(); const store = options.store; const boundFns = new Map(); const _TAG = this[IDENTIFIER_ARRAY_TAG]; diff --git a/packages/tracking/addon-main.js b/packages/tracking/addon-main.js index 459ef9174ca..13f812d930a 100644 --- a/packages/tracking/addon-main.js +++ b/packages/tracking/addon-main.js @@ -1,5 +1,79 @@ +const requireModule = require('@ember-data/private-build-infra/src/utilities/require-module'); +const getEnv = require('@ember-data/private-build-infra/src/utilities/get-env'); +const detectModule = require('@ember-data/private-build-infra/src/utilities/detect-module'); + +const pkg = require('./package.json'); + module.exports = { - name: require('./package.json').name, + name: pkg.name, + + options: { + '@embroider/macros': { + setOwnConfig: {}, + }, + }, + + _emberDataConfig: null, + configureEmberData() { + if (this._emberDataConfig) { + return this._emberDataConfig; + } + const app = this._findHost(); + const isProd = /production/.test(process.env.EMBER_ENV); + const hostOptions = app.options?.emberData || {}; + const debugOptions = Object.assign( + { + LOG_PAYLOADS: false, + LOG_OPERATIONS: false, + LOG_MUTATIONS: false, + LOG_NOTIFICATIONS: false, + LOG_REQUESTS: false, + LOG_REQUEST_STATUS: false, + LOG_IDENTIFIERS: false, + LOG_GRAPH: false, + LOG_INSTANCE_CACHE: false, + }, + hostOptions.debug || {} + ); + + const HAS_DEBUG_PACKAGE = detectModule(require, '@ember-data/debug', __dirname, pkg); + const HAS_META_PACKAGE = detectModule(require, 'ember-data', __dirname, pkg); + + const includeDataAdapterInProduction = + typeof hostOptions.includeDataAdapterInProduction === 'boolean' + ? hostOptions.includeDataAdapterInProduction + : HAS_META_PACKAGE; + + const includeDataAdapter = HAS_DEBUG_PACKAGE ? (isProd ? includeDataAdapterInProduction : true) : false; + const DEPRECATIONS = require('@ember-data/private-build-infra/src/deprecations')(hostOptions.compatWith || null); + const FEATURES = require('@ember-data/private-build-infra/src/features')(isProd); + + const ALL_PACKAGES = requireModule('@ember-data/private-build-infra/virtual-packages/packages.js'); + const MACRO_PACKAGE_FLAGS = Object.assign({}, ALL_PACKAGES.default); + delete MACRO_PACKAGE_FLAGS['HAS_DEBUG_PACKAGE']; + + Object.keys(MACRO_PACKAGE_FLAGS).forEach((key) => { + MACRO_PACKAGE_FLAGS[key] = detectModule(require, MACRO_PACKAGE_FLAGS[key], __dirname, pkg); + }); + + // copy configs forward + const ownConfig = this.options['@embroider/macros'].setOwnConfig; + ownConfig.compatWith = hostOptions.compatWith || null; + ownConfig.debug = debugOptions; + ownConfig.deprecations = Object.assign(DEPRECATIONS, ownConfig.deprecations || {}, hostOptions.deprecations || {}); + ownConfig.features = Object.assign({}, FEATURES, ownConfig.features || {}, hostOptions.features || {}); + ownConfig.includeDataAdapter = includeDataAdapter; + ownConfig.packages = MACRO_PACKAGE_FLAGS; + ownConfig.env = getEnv(ownConfig); + + this._emberDataConfig = ownConfig; + return ownConfig; + }, + + included() { + this.configureEmberData(); + return this._super.included.call(this, ...arguments); + }, treeForVendor() { return; diff --git a/packages/tracking/babel.config.js b/packages/tracking/babel.config.js new file mode 100644 index 00000000000..944b6102d62 --- /dev/null +++ b/packages/tracking/babel.config.js @@ -0,0 +1,13 @@ +const macros = require('@ember-data/private-build-infra/src/v2-babel-build-pack'); + +module.exports = { + plugins: [ + ...macros, + // '@embroider/macros/src/babel/macros-babel-plugin.js', + ['@babel/plugin-transform-runtime', { loose: true }], + ['@babel/plugin-transform-typescript', { allowDeclareFields: true }], + ['@babel/plugin-proposal-decorators', { legacy: true, loose: true }], + ['@babel/plugin-proposal-private-methods', { loose: true }], + ['@babel/plugin-proposal-class-properties', { loose: true }], + ], +}; diff --git a/packages/tracking/babel.config.json b/packages/tracking/babel.config.json deleted file mode 100644 index fea1b66f091..00000000000 --- a/packages/tracking/babel.config.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "plugins": [ - "@babel/plugin-transform-runtime", - ["@babel/plugin-transform-typescript", { "allowDeclareFields": true }], - ["@babel/plugin-proposal-decorators", { "legacy": true }], - "@babel/plugin-proposal-class-properties" - ] -} diff --git a/packages/tracking/package.json b/packages/tracking/package.json index be59d6225c6..c293351b471 100644 --- a/packages/tracking/package.json +++ b/packages/tracking/package.json @@ -21,8 +21,15 @@ "volta": { "extends": "../../package.json" }, + "dependenciesMeta": { + "@ember-data/private-build-infra": { + "injected": true + } + }, "dependencies": { - "ember-cli-babel": "^7.26.11" + "ember-cli-babel": "^7.26.11", + "@ember-data/private-build-infra": "workspace:4.12.1", + "@embroider/macros": "^1.10.0" }, "files": [ "addon-main.js", @@ -47,6 +54,7 @@ "@babel/core": "^7.21.4", "@babel/cli": "^7.21.0", "@babel/plugin-proposal-class-properties": "^7.18.6", + "@babel/plugin-proposal-private-methods": "^7.18.6", "@babel/plugin-proposal-decorators": "^7.21.0", "@babel/plugin-transform-runtime": "^7.21.4", "@babel/plugin-transform-typescript": "^7.21.3", diff --git a/packages/tracking/rollup.config.mjs b/packages/tracking/rollup.config.mjs index 8a728409277..34fe8a317de 100644 --- a/packages/tracking/rollup.config.mjs +++ b/packages/tracking/rollup.config.mjs @@ -12,7 +12,7 @@ export default { // You can augment this if you need to. output: addon.output(), - external: [], + external: ['@embroider/macros'], plugins: [ // These are the modules that users should be able to import from your diff --git a/packages/tracking/src/-private.ts b/packages/tracking/src/-private.ts index 812b43c3c97..a1bedd43ad1 100644 --- a/packages/tracking/src/-private.ts +++ b/packages/tracking/src/-private.ts @@ -1,3 +1,5 @@ +import { DEBUG } from '@ember-data/env'; + /** * This package provides primitives that allow powerful low-level * adjustments to change tracking notification behaviors. @@ -43,6 +45,62 @@ export function subscribe(obj: Tag): void { } } +function updateRef(obj: Tag): void { + if (DEBUG) { + try { + obj.ref = null; + } catch (e: unknown) { + if (e instanceof Error) { + if (e.message.includes('You attempted to update `ref` on `Tag`')) { + e.message = e.message.replace( + 'You attempted to update `ref` on `Tag`', + // @ts-expect-error + `You attempted to update <${obj._debug_base}>.${obj._debug_prop}` // eslint-disable-line + ); + e.stack = e.stack?.replace( + 'You attempted to update `ref` on `Tag`', + // @ts-expect-error + `You attempted to update <${obj._debug_base}>.${obj._debug_prop}` // eslint-disable-line + ); + + const lines = e.stack?.split(`\n`); + const finalLines: string[] = []; + let lastFile: string | null = null; + + lines?.forEach((line) => { + if (line.trim().startsWith('at ')) { + // get the last string in the line which contains the code source location + const location = line.split(' ').at(-1)!; + // remove the line and char offset info + + if (location.includes(':')) { + const parts = location.split(':'); + parts.pop(); + parts.pop(); + const file = parts.join(':'); + if (file !== lastFile) { + lastFile = file; + finalLines.push(''); + } + } + finalLines.push(line); + } + }); + + const splitstr = '`ref` was first used:'; + const parts = e.message.split(splitstr); + parts.splice(1, 0, `Original Stack\n=============\n${finalLines.join(`\n`)}\n\n${splitstr}`); + + e.message = parts.join(''); + } + } + throw e; + } + } else { + obj.ref = null; + } +} + function flushTransaction() { let transaction = TRANSACTION!; TRANSACTION = transaction.parent; @@ -52,7 +110,7 @@ function flushTransaction() { transaction.props.forEach((obj: Tag) => { // mark this mutation as part of a transaction obj.t = true; - obj.ref = null; + updateRef(obj); }); transaction.sub.forEach((obj: Tag) => { obj.ref; @@ -70,7 +128,7 @@ async function untrack() { transaction.props.forEach((obj: Tag) => { // mark this mutation as part of a transaction obj.t = true; - obj.ref = null; + updateRef(obj); }); } @@ -78,7 +136,7 @@ export function addToTransaction(obj: Tag): void { if (TRANSACTION) { TRANSACTION.props.add(obj); } else { - obj.ref = null; + updateRef(obj); } } export function addTransactionCB(method: OpaqueFn): void { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 4409a54406e..ad06dfe8222 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1031,6 +1031,12 @@ importers: packages/tracking: dependencies: + '@ember-data/private-build-infra': + specifier: workspace:4.12.1 + version: file:packages/private-build-infra + '@embroider/macros': + specifier: 1.10.0 + version: 1.10.0 ember-cli-babel: specifier: ^7.26.11 version: 7.26.11 @@ -1047,6 +1053,9 @@ importers: '@babel/plugin-proposal-decorators': specifier: ^7.21.0 version: 7.21.0(@babel/core@7.21.4) + '@babel/plugin-proposal-private-methods': + specifier: ^7.18.6 + version: 7.18.6(@babel/core@7.21.4) '@babel/plugin-transform-runtime': specifier: ^7.21.4 version: 7.21.4(@babel/core@7.21.4) @@ -1083,6 +1092,9 @@ importers: walk-sync: specifier: ^3.0.0 version: 3.0.0 + dependenciesMeta: + '@ember-data/private-build-infra': + injected: true packages/unpublished-eslint-rules: {} @@ -16738,6 +16750,8 @@ packages: version: 4.12.1 engines: {node: 16.* || >= 18} dependencies: + '@ember-data/private-build-infra': file:packages/private-build-infra + '@embroider/macros': 1.10.0 ember-cli-babel: 7.26.11 transitivePeerDependencies: - supports-color diff --git a/tests/main/tests/end-user-dx/meaningful-backtracking-errors-test.js b/tests/main/tests/end-user-dx/meaningful-backtracking-errors-test.js new file mode 100644 index 00000000000..1a4848a6af6 --- /dev/null +++ b/tests/main/tests/end-user-dx/meaningful-backtracking-errors-test.js @@ -0,0 +1,51 @@ +import { render, setupOnerror } from '@ember/test-helpers'; + +import hbs from 'htmlbars-inline-precompile'; +import { module, test } from 'qunit'; + +import Store from 'ember-data/store'; +import { setupRenderingTest } from 'ember-qunit'; + +import Model, { attr, belongsTo, hasMany } from '@ember-data/model'; + +module('DX | Meaningful Backtracking Errors', function (hooks) { + setupRenderingTest(hooks); + + hooks.beforeEach(function () { + this.owner.register('service:store', Store); + this.owner.register( + 'model:user', + class extends Model { + @attr name; + } + ); + }); + + test('We meaningfully error for live arrays', async function (assert) { + assert.expect(1); + const store = this.owner.lookup('service:store'); + + class PoorlyWrittenCode { + get value() { + return store.createRecord('user', { name: 'Chris' }); + } + } + + this.set('records', store.peekAll('user')); + this.set('badCode', new PoorlyWrittenCode()); + + function handler(error) { + assert.strictEqual(error.message, '', 'we have a meaningful error'); + return false; + } + + // setupOnerror(handler); + + await render(hbs` + Count: {{this.records.length}} + Value: {{this.badCode.value}} + `); + + // setupOnerror(); + }); +}); From 119701d0fa95a096e55f641fa5c545bf43a90393 Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Fri, 30 Jun 2023 16:58:42 -0700 Subject: [PATCH 2/3] fixes --- .../meaningful-backtracking-errors-test.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/main/tests/end-user-dx/meaningful-backtracking-errors-test.js b/tests/main/tests/end-user-dx/meaningful-backtracking-errors-test.js index 1a4848a6af6..c7922d0a7f2 100644 --- a/tests/main/tests/end-user-dx/meaningful-backtracking-errors-test.js +++ b/tests/main/tests/end-user-dx/meaningful-backtracking-errors-test.js @@ -6,7 +6,7 @@ import { module, test } from 'qunit'; import Store from 'ember-data/store'; import { setupRenderingTest } from 'ember-qunit'; -import Model, { attr, belongsTo, hasMany } from '@ember-data/model'; +import Model, { attr } from '@ember-data/model'; module('DX | Meaningful Backtracking Errors', function (hooks) { setupRenderingTest(hooks); @@ -35,17 +35,22 @@ module('DX | Meaningful Backtracking Errors', function (hooks) { this.set('badCode', new PoorlyWrittenCode()); function handler(error) { - assert.strictEqual(error.message, '', 'we have a meaningful error'); + assert.true( + error.message.includes( + 'You attempted to update .length, but it had already been used previously in the same computation' + ), + 'we have a meaningful error' + ); return false; } - // setupOnerror(handler); + setupOnerror(handler); await render(hbs` Count: {{this.records.length}} Value: {{this.badCode.value}} `); - // setupOnerror(); + setupOnerror(); }); }); From e6c30b1ced9195fbf54403b44525078337377dc1 Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Fri, 30 Jun 2023 17:07:30 -0700 Subject: [PATCH 3/3] fix prod test --- .../tests/end-user-dx/meaningful-backtracking-errors-test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/main/tests/end-user-dx/meaningful-backtracking-errors-test.js b/tests/main/tests/end-user-dx/meaningful-backtracking-errors-test.js index c7922d0a7f2..e24968a051b 100644 --- a/tests/main/tests/end-user-dx/meaningful-backtracking-errors-test.js +++ b/tests/main/tests/end-user-dx/meaningful-backtracking-errors-test.js @@ -1,12 +1,13 @@ import { render, setupOnerror } from '@ember/test-helpers'; import hbs from 'htmlbars-inline-precompile'; -import { module, test } from 'qunit'; +import { module } from 'qunit'; import Store from 'ember-data/store'; import { setupRenderingTest } from 'ember-qunit'; import Model, { attr } from '@ember-data/model'; +import test from '@ember-data/unpublished-test-infra/test-support/test-in-debug'; module('DX | Meaningful Backtracking Errors', function (hooks) { setupRenderingTest(hooks);