From d264bf47bc73b202c8fc8f2b62d338d705364541 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Fri, 23 Aug 2024 15:24:47 -0400 Subject: [PATCH 01/17] working to remove circular dependencies --- packages/@ember/array/index.ts | 2 +- .../array/{lib/make-array.ts => make.ts} | 0 packages/@ember/array/package.json | 1 + packages/@ember/object/core.ts | 2 +- packages/@ember/object/observable.ts | 6 +++++- .../@ember/routing/lib/routing-service.ts | 2 +- packages/@ember/routing/route.ts | 17 +++++----------- packages/@ember/routing/router-service.ts | 2 +- packages/@ember/routing/router.ts | 10 ++++++---- rollup.config.mjs | 20 ++++++++++++++++++- 10 files changed, 40 insertions(+), 22 deletions(-) rename packages/@ember/array/{lib/make-array.ts => make.ts} (100%) diff --git a/packages/@ember/array/index.ts b/packages/@ember/array/index.ts index ac6130fd320..80e6acda66a 100644 --- a/packages/@ember/array/index.ts +++ b/packages/@ember/array/index.ts @@ -22,7 +22,7 @@ import type { MethodNamesOf, MethodParams, MethodReturns } from '@ember/-interna import type { ComputedPropertyCallback } from '@ember/-internals/metal'; import { isEmberArray, setEmberArray } from '@ember/array/-internals'; -export { default as makeArray } from './lib/make-array'; +export { default as makeArray } from './make'; export type EmberArrayLike = EmberArray | NativeArray; diff --git a/packages/@ember/array/lib/make-array.ts b/packages/@ember/array/make.ts similarity index 100% rename from packages/@ember/array/lib/make-array.ts rename to packages/@ember/array/make.ts diff --git a/packages/@ember/array/package.json b/packages/@ember/array/package.json index 1f77264331c..6aec1c8c083 100644 --- a/packages/@ember/array/package.json +++ b/packages/@ember/array/package.json @@ -6,6 +6,7 @@ ".": "./index.ts", "./-internals": "./-internals.ts", "./proxy": "./proxy.ts", + "./make": "./make.ts", "./mutable": "./mutable.ts" }, "dependencies": { diff --git a/packages/@ember/object/core.ts b/packages/@ember/object/core.ts index 9eb4fcd509e..330a1d667e4 100644 --- a/packages/@ember/object/core.ts +++ b/packages/@ember/object/core.ts @@ -19,7 +19,7 @@ import { } from '@ember/-internals/metal'; import Mixin, { applyMixin } from '@ember/object/mixin'; import { ActionHandler } from '@ember/-internals/runtime'; -import { makeArray } from '@ember/array'; +import makeArray from '@ember/array/make'; import { assert } from '@ember/debug'; import { DEBUG } from '@glimmer/env'; import { destroy, isDestroying, isDestroyed, registerDestructor } from '@glimmer/destroyable'; diff --git a/packages/@ember/object/observable.ts b/packages/@ember/object/observable.ts index fb760925efc..05ba879bac8 100644 --- a/packages/@ember/object/observable.ts +++ b/packages/@ember/object/observable.ts @@ -10,8 +10,12 @@ import { endPropertyChanges, addObserver, removeObserver, + get, + set, + getProperties, + setProperties, } from '@ember/-internals/metal'; -import { get, set, getProperties, setProperties } from '@ember/object'; + import Mixin from '@ember/object/mixin'; import { assert } from '@ember/debug'; diff --git a/packages/@ember/routing/lib/routing-service.ts b/packages/@ember/routing/lib/routing-service.ts index a3d4758bbe5..610fcdac4e9 100644 --- a/packages/@ember/routing/lib/routing-service.ts +++ b/packages/@ember/routing/lib/routing-service.ts @@ -9,7 +9,7 @@ import Service from '@ember/service'; import type { ModelFor } from 'router_js'; import type Route from '@ember/routing/route'; import EmberRouter from '@ember/routing/router'; -import type { RouterState } from '@ember/routing/-internals'; +import type RouterState from './router_state'; import { ROUTER } from '@ember/routing/router-service'; /** diff --git a/packages/@ember/routing/route.ts b/packages/@ember/routing/route.ts index 62b51e9c6c8..c407f629caf 100644 --- a/packages/@ember/routing/route.ts +++ b/packages/@ember/routing/route.ts @@ -8,7 +8,7 @@ import { import type Owner from '@ember/owner'; import { getOwner } from '@ember/-internals/owner'; import { ENV } from '@ember/-internals/environment'; -import { BucketCache } from '@ember/routing/-internals'; +import type { default as BucketCache } from './lib/cache'; import EmberObject, { computed, get, set, getProperties, setProperties } from '@ember/object'; import Evented from '@ember/object/evented'; import { A as emberA } from '@ember/array'; @@ -28,9 +28,8 @@ import type { RenderState } from '@ember/-internals/glimmer'; import type { TemplateFactory } from '@glimmer/interfaces'; import type { InternalRouteInfo, Route as IRoute, Transition, TransitionState } from 'router_js'; import { PARAMS_SYMBOL, STATE_SYMBOL } from 'router_js'; -import type { QueryParam } from '@ember/routing/router'; -import EmberRouter from '@ember/routing/router'; -import { generateController } from '@ember/routing/-internals'; +import type { QueryParam, default as EmberRouter } from '@ember/routing/router'; +import { default as generateController } from './lib/generate_controller'; import type { ExpandedControllerQueryParam, NamedRouteArgs } from './lib/utils'; import { calculateCacheKey, @@ -293,14 +292,8 @@ class Route extends EmberObject.extend(ActionHandler, Evented) if (owner) { let router = owner.lookup('router:main'); let bucketCache = owner.lookup(P`-bucket-cache:main`); - - assert( - 'ROUTER BUG: Expected route injections to be defined on the route. This is an internal bug, please open an issue on Github if you see this message!', - router instanceof EmberRouter && bucketCache instanceof BucketCache - ); - - this._router = router; - this._bucketCache = bucketCache; + this._router = router as EmberRouter; + this._bucketCache = bucketCache as BucketCache; this._topLevelViewTemplate = owner.lookup('template:-outlet'); this._environment = owner.lookup('-environment:main'); } diff --git a/packages/@ember/routing/router-service.ts b/packages/@ember/routing/router-service.ts index 2dab52c3670..f99166ae6b7 100644 --- a/packages/@ember/routing/router-service.ts +++ b/packages/@ember/routing/router-service.ts @@ -10,7 +10,7 @@ import { consumeTag, tagFor } from '@glimmer/validator'; import type { ModelFor, Transition } from 'router_js'; import type Route from '@ember/routing/route'; import EmberRouter from '@ember/routing/router'; -import type { RouteInfo, RouteInfoWithAttributes } from '@ember/routing/-internals'; +import type { RouteInfo, RouteInfoWithAttributes } from './lib/route-info'; import type { RouteArgs, RouteOptions } from './lib/utils'; import { extractRouteArgs, resemblesURL, shallowEqual } from './lib/utils'; diff --git a/packages/@ember/routing/router.ts b/packages/@ember/routing/router.ts index f41a58d6a4e..9de54e2c662 100644 --- a/packages/@ember/routing/router.ts +++ b/packages/@ember/routing/router.ts @@ -3,8 +3,10 @@ import type { BootEnvironment, OutletState, OutletView } from '@ember/-internals import { computed, get, set } from '@ember/object'; import type { default as Owner, FactoryManager } from '@ember/owner'; import { getOwner } from '@ember/owner'; -import { BucketCache, DSL, RouterState } from '@ember/routing/-internals'; -import type { DSLCallback, EngineRouteInfo } from '@ember/routing/-internals'; +import { default as BucketCache } from './lib/cache'; +import { default as DSL, type DSLCallback } from './lib/dsl'; +import RouterState from './lib/router_state'; +import type { EngineRouteInfo } from './lib/engines'; import { calculateCacheKey, extractRouteArgs, @@ -24,9 +26,9 @@ import Evented from '@ember/object/evented'; import { assert, info } from '@ember/debug'; import { cancel, once, run, scheduleOnce } from '@ember/runloop'; import { DEBUG } from '@glimmer/env'; -import type { QueryParamMeta } from '@ember/routing/route'; -import type Route from '@ember/routing/route'; import { + type QueryParamMeta, + type default as Route, defaultSerialize, getFullQueryParams, getRenderState, diff --git a/rollup.config.mjs b/rollup.config.mjs index dae34307ff7..c63ce22f637 100644 --- a/rollup.config.mjs +++ b/rollup.config.mjs @@ -16,7 +16,7 @@ const canaryFeatures = require('./broccoli/canary-features'); const testDependencies = ['qunit', 'vite']; -export default [ +let configs = [ esmConfig(), legacyBundleConfig('./broccoli/amd-compat-entrypoints/ember.debug.js', 'ember.debug.js', { isDeveloping: true, @@ -33,6 +33,12 @@ export default [ templateCompilerConfig(), ]; +if (process.env.DEBUG_SINGLE_CONFIG) { + configs = configs.slice(parseInt(process.env.DEBUG_SINGLE_CONFIG), 1); +} + +export default configs; + function esmConfig() { let babelConfig = { ...sharedBabelConfig }; babelConfig.plugins = [ @@ -42,6 +48,18 @@ function esmConfig() { ]; return { + onLog(level, log, handler) { + switch (log.code) { + case 'CIRCULAR_DEPENDENCY': + process.stderr.write(log.message + '\n'); + break; + case 'CYCLIC_CROSS_CHUNK_REEXPORT': + case 'EMPTY_BUNDLE': + break; + default: + handler(level, log); + } + }, input: { ...renameEntrypoints(exposedDependencies(), (name) => join('packages', name, 'index')), ...renameEntrypoints(packages(), (name) => join('packages', name)), From 4ae85638542facf71642d59e97924dcb3566f5bd Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Mon, 26 Aug 2024 09:38:42 -0400 Subject: [PATCH 02/17] keep lib/make-array because it's present in the classic AMD bundles --- packages/@ember/array/lib/make-array.ts | 40 ++++++++++++++++++++++++ packages/@ember/array/make.ts | 41 +------------------------ 2 files changed, 41 insertions(+), 40 deletions(-) create mode 100644 packages/@ember/array/lib/make-array.ts diff --git a/packages/@ember/array/lib/make-array.ts b/packages/@ember/array/lib/make-array.ts new file mode 100644 index 00000000000..12f628eed78 --- /dev/null +++ b/packages/@ember/array/lib/make-array.ts @@ -0,0 +1,40 @@ +const { isArray } = Array; +/** + @module @ember/array +*/ +/** + Forces the passed object to be part of an array. If the object is already + an array, it will return the object. Otherwise, it will add the object to + an array. If object is `null` or `undefined`, it will return an empty array. + + ```javascript + import { makeArray } from '@ember/array'; + import ArrayProxy from '@ember/array/proxy'; + + makeArray(); // [] + makeArray(null); // [] + makeArray(undefined); // [] + makeArray('lindsay'); // ['lindsay'] + makeArray([1, 2, 42]); // [1, 2, 42] + + let proxy = ArrayProxy.create({ content: [] }); + + makeArray(proxy) === proxy; // false + ``` + + @method makeArray + @static + @for @ember/array + @param {Object} obj the object + @return {Array} + @private + */ +function makeArray(obj: T): T extends TT[] ? T : T extends null | undefined ? [] : [T]; +function makeArray(obj: any | null | undefined): Array { + if (obj === null || obj === undefined) { + return []; + } + return isArray(obj) ? obj : [obj]; +} + +export default makeArray; diff --git a/packages/@ember/array/make.ts b/packages/@ember/array/make.ts index 12f628eed78..3ab2b4caa48 100644 --- a/packages/@ember/array/make.ts +++ b/packages/@ember/array/make.ts @@ -1,40 +1 @@ -const { isArray } = Array; -/** - @module @ember/array -*/ -/** - Forces the passed object to be part of an array. If the object is already - an array, it will return the object. Otherwise, it will add the object to - an array. If object is `null` or `undefined`, it will return an empty array. - - ```javascript - import { makeArray } from '@ember/array'; - import ArrayProxy from '@ember/array/proxy'; - - makeArray(); // [] - makeArray(null); // [] - makeArray(undefined); // [] - makeArray('lindsay'); // ['lindsay'] - makeArray([1, 2, 42]); // [1, 2, 42] - - let proxy = ArrayProxy.create({ content: [] }); - - makeArray(proxy) === proxy; // false - ``` - - @method makeArray - @static - @for @ember/array - @param {Object} obj the object - @return {Array} - @private - */ -function makeArray(obj: T): T extends TT[] ? T : T extends null | undefined ? [] : [T]; -function makeArray(obj: any | null | undefined): Array { - if (obj === null || obj === undefined) { - return []; - } - return isArray(obj) ? obj : [obj]; -} - -export default makeArray; +export { default } from './lib/make-array'; From f888659d9ef912a00afeda043bd8ae8eb878c289 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Mon, 26 Aug 2024 10:45:06 -0400 Subject: [PATCH 03/17] fixing more cycles --- packages/@ember/-internals/views/lib/system/event_dispatcher.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@ember/-internals/views/lib/system/event_dispatcher.ts b/packages/@ember/-internals/views/lib/system/event_dispatcher.ts index 1e281c9bcaf..db3512393fc 100644 --- a/packages/@ember/-internals/views/lib/system/event_dispatcher.ts +++ b/packages/@ember/-internals/views/lib/system/event_dispatcher.ts @@ -2,7 +2,7 @@ import { getOwner } from '@ember/-internals/owner'; import { assert } from '@ember/debug'; import { get, set } from '@ember/-internals/metal'; import EmberObject from '@ember/object'; -import { getElementView } from '@ember/-internals/views'; +import { getElementView } from './utils'; import ActionManager from './action_manager'; import type { BootEnvironment } from '@ember/-internals/glimmer/lib/views/outlet'; import type Component from '@ember/component'; From 8f3b9f715bca395dd923030fd519d2b8fe628e3a Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Mon, 26 Aug 2024 10:49:28 -0400 Subject: [PATCH 04/17] Fix cycle This assertion was only added to satisfy typescript. It creates a module cycle and isn't worth it. --- packages/@ember/engine/instance.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/@ember/engine/instance.ts b/packages/@ember/engine/instance.ts index 4fa6c2fb71f..1565b8eca83 100644 --- a/packages/@ember/engine/instance.ts +++ b/packages/@ember/engine/instance.ts @@ -12,7 +12,7 @@ import { ContainerProxyMixin, RegistryProxyMixin } from '@ember/-internals/runti import type { InternalOwner } from '@ember/-internals/owner'; import type Owner from '@ember/-internals/owner'; import { type FullName, isFactory } from '@ember/-internals/owner'; -import Engine from '@ember/engine'; +import type Engine from '@ember/engine'; import type Application from '@ember/application'; import type { BootEnvironment } from '@ember/-internals/glimmer'; import type { SimpleElement } from '@simple-dom/interface'; @@ -199,7 +199,7 @@ class EngineInstance extends EmberObject.extend(RegistryProxyMixin, ContainerPro @return {EngineInstance,Error} */ buildChildEngineInstance(name: string, options: EngineInstanceOptions = {}): EngineInstance { - let ChildEngine = this.lookup(`engine:${name}`); + let ChildEngine = this.lookup(`engine:${name}`) as Engine; if (!ChildEngine) { throw new Error( @@ -207,8 +207,6 @@ class EngineInstance extends EmberObject.extend(RegistryProxyMixin, ContainerPro ); } - assert('expected an Engine', ChildEngine instanceof Engine); - let engineInstance = ChildEngine.buildInstance(options); setEngineParent(engineInstance, this); From 434fda03dced052639ba06c7411d8c94c4e3df31 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Mon, 26 Aug 2024 12:46:03 -0400 Subject: [PATCH 05/17] fixing cycles around `assert` --- packages/@ember/debug/index.ts | 57 ++----------------------- packages/@ember/debug/lib/assert.ts | 59 ++++++++++++++++++++++++++ packages/@ember/debug/lib/deprecate.ts | 2 +- packages/@ember/debug/lib/inspect.ts | 2 +- packages/@ember/debug/lib/warn.ts | 2 +- 5 files changed, 66 insertions(+), 56 deletions(-) create mode 100644 packages/@ember/debug/lib/assert.ts diff --git a/packages/@ember/debug/index.ts b/packages/@ember/debug/index.ts index 9dbf41386d0..77f34ecad51 100644 --- a/packages/@ember/debug/index.ts +++ b/packages/@ember/debug/index.ts @@ -6,6 +6,7 @@ import defaultDeprecate from './lib/deprecate'; import { isTesting } from './lib/testing'; import type { WarnFunc } from './lib/warn'; import _warn from './lib/warn'; +import { assert, setAssert } from './lib/assert'; export { registerHandler as registerWarnHandler } from './lib/warn'; export { @@ -27,10 +28,6 @@ export type DebugFunctionType = | 'runInDebug' | 'deprecateFunc'; -export interface AssertFunc { - (desc: string, condition: unknown): asserts condition; - (desc: string): never; -} export type DebugFunc = (message: string) => void; export type DebugSealFunc = (obj: object) => void; export type DebugFreezeFunc = (obj: object) => void; @@ -43,7 +40,7 @@ export type DeprecateFuncFunc = ( ) => Function; export type GetDebugFunction = { - (type: 'assert'): AssertFunc; + (type: 'assert'): typeof assert; (type: 'info'): InfoFunc; (type: 'warn'): WarnFunc; (type: 'debug'): DebugFunc; @@ -55,7 +52,7 @@ export type GetDebugFunction = { }; export type SetDebugFunction = { - (type: 'assert', func: AssertFunc): AssertFunc; + (type: 'assert', func: typeof assert): typeof assert; (type: 'info', func: InfoFunc): InfoFunc; (type: 'warn', func: WarnFunc): WarnFunc; (type: 'debug', func: DebugFunc): DebugFunc; @@ -71,7 +68,6 @@ const noop = () => {}; // SAFETY: these casts are just straight-up lies, but the point is that they do // not do anything in production builds. -let assert: AssertFunc = noop as unknown as AssertFunc; let info: InfoFunc = noop; let warn: WarnFunc = noop; let debug: DebugFunc = noop; @@ -94,7 +90,7 @@ if (DEBUG) { setDebugFunction = function (type: DebugFunctionType, callback: Function) { switch (type) { case 'assert': - return (assert = callback as AssertFunc); + return setAssert(callback as typeof assert); case 'info': return (info = callback as InfoFunc); case 'warn': @@ -143,51 +139,6 @@ if (DEBUG) { */ if (DEBUG) { - /** - Verify that a certain expectation is met, or throw a exception otherwise. - - This is useful for communicating assumptions in the code to other human - readers as well as catching bugs that accidentally violates these - expectations. - - Assertions are removed from production builds, so they can be freely added - for documentation and debugging purposes without worries of incuring any - performance penalty. However, because of that, they should not be used for - checks that could reasonably fail during normal usage. Furthermore, care - should be taken to avoid accidentally relying on side-effects produced from - evaluating the condition itself, since the code will not run in production. - - ```javascript - import { assert } from '@ember/debug'; - - // Test for truthiness - assert('Must pass a string', typeof str === 'string'); - - // Fail unconditionally - assert('This code path should never be run'); - ``` - - @method assert - @static - @for @ember/debug - @param {String} description Describes the expectation. This will become the - text of the Error thrown if the assertion fails. - @param {any} condition Must be truthy for the assertion to pass. If - falsy, an exception will be thrown. - @public - @since 1.0.0 - */ - function assert(desc: string): never; - function assert(desc: string, test: unknown): asserts test; - // eslint-disable-next-line no-inner-declarations - function assert(desc: string, test?: unknown): asserts test { - if (!test) { - throw new Error(`Assertion Failed: ${desc}`); - } - } - - setDebugFunction('assert', assert); - /** Display a debug notice. diff --git a/packages/@ember/debug/lib/assert.ts b/packages/@ember/debug/lib/assert.ts new file mode 100644 index 00000000000..1c1a00184b1 --- /dev/null +++ b/packages/@ember/debug/lib/assert.ts @@ -0,0 +1,59 @@ +import { DEBUG } from '@glimmer/env'; + +export interface AssertFunc { + (desc: string, condition: unknown): asserts condition; + (desc: string): never; +} + +export let assert: AssertFunc = (() => {}) as unknown as AssertFunc; + +export function setAssert(implementation: typeof assert): typeof assert { + assert = implementation; + return implementation; +} + +if (DEBUG) { + /** + Verify that a certain expectation is met, or throw a exception otherwise. + + This is useful for communicating assumptions in the code to other human + readers as well as catching bugs that accidentally violates these + expectations. + + Assertions are removed from production builds, so they can be freely added + for documentation and debugging purposes without worries of incuring any + performance penalty. However, because of that, they should not be used for + checks that could reasonably fail during normal usage. Furthermore, care + should be taken to avoid accidentally relying on side-effects produced from + evaluating the condition itself, since the code will not run in production. + + ```javascript + import { assert } from '@ember/debug'; + + // Test for truthiness + assert('Must pass a string', typeof str === 'string'); + + // Fail unconditionally + assert('This code path should never be run'); + ``` + + @method assert + @static + @for @ember/debug + @param {String} description Describes the expectation. This will become the + text of the Error thrown if the assertion fails. + @param {any} condition Must be truthy for the assertion to pass. If + falsy, an exception will be thrown. + @public + @since 1.0.0 + */ + function assert(desc: string): never; + function assert(desc: string, test: unknown): asserts test; + // eslint-disable-next-line no-inner-declarations + function assert(desc: string, test?: unknown): asserts test { + if (!test) { + throw new Error(`Assertion Failed: ${desc}`); + } + } + setAssert(assert); +} diff --git a/packages/@ember/debug/lib/deprecate.ts b/packages/@ember/debug/lib/deprecate.ts index c647d6ba95d..20a30302868 100644 --- a/packages/@ember/debug/lib/deprecate.ts +++ b/packages/@ember/debug/lib/deprecate.ts @@ -1,7 +1,7 @@ import { ENV } from '@ember/-internals/environment'; import { DEBUG } from '@glimmer/env'; -import { assert } from '../index'; +import { assert } from './assert'; import type { HandlerCallback } from './handlers'; import { invoke, registerHandler as genericRegisterHandler } from './handlers'; diff --git a/packages/@ember/debug/lib/inspect.ts b/packages/@ember/debug/lib/inspect.ts index 6dfe664ed91..2063e300f7c 100644 --- a/packages/@ember/debug/lib/inspect.ts +++ b/packages/@ember/debug/lib/inspect.ts @@ -1,4 +1,4 @@ -import { assert } from '@ember/debug'; +import { assert } from './assert'; const { toString: objectToString } = Object.prototype; const { toString: functionToString } = Function.prototype; const { isArray } = Array; diff --git a/packages/@ember/debug/lib/warn.ts b/packages/@ember/debug/lib/warn.ts index ad8bda9bc38..0a048571270 100644 --- a/packages/@ember/debug/lib/warn.ts +++ b/packages/@ember/debug/lib/warn.ts @@ -1,6 +1,6 @@ import { DEBUG } from '@glimmer/env'; -import { assert } from '../index'; +import { assert } from './assert'; import type { HandlerCallback } from './handlers'; import { invoke, registerHandler as genericRegisterHandler } from './handlers'; From c00789537b0b7e4d7010abe3ee58d46ef3dc36fa Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Mon, 26 Aug 2024 15:31:56 -0400 Subject: [PATCH 06/17] silence rsvp warnings --- rollup.config.mjs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rollup.config.mjs b/rollup.config.mjs index c63ce22f637..ed152719db9 100644 --- a/rollup.config.mjs +++ b/rollup.config.mjs @@ -51,6 +51,10 @@ function esmConfig() { onLog(level, log, handler) { switch (log.code) { case 'CIRCULAR_DEPENDENCY': + if (log.ids.some((id) => id.includes('node_modules/rsvp/lib/rsvp'))) { + // rsvp has some internal cycles but they don't bother us + return; + } process.stderr.write(log.message + '\n'); break; case 'CYCLIC_CROSS_CHUNK_REEXPORT': From baa6bf438e9bbe84d5438c2c3352019c9d875c25 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Mon, 26 Aug 2024 15:44:23 -0400 Subject: [PATCH 07/17] improved debug printing --- rollup.config.mjs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rollup.config.mjs b/rollup.config.mjs index ed152719db9..901b2a3ad7e 100644 --- a/rollup.config.mjs +++ b/rollup.config.mjs @@ -55,7 +55,9 @@ function esmConfig() { // rsvp has some internal cycles but they don't bother us return; } - process.stderr.write(log.message + '\n'); + process.stderr.write( + `Circular dependency:\n${log.ids.map((id) => ' ' + id).join('\n')}\n` + ); break; case 'CYCLIC_CROSS_CHUNK_REEXPORT': case 'EMPTY_BUNDLE': From 87bd25265de68f289170d102318d872654de34c9 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Mon, 26 Aug 2024 15:44:42 -0400 Subject: [PATCH 08/17] fixing another cycle --- .../@ember/-internals/glimmer/lib/component-managers/curly.ts | 1 + packages/@ember/-internals/glimmer/lib/utils/process-args.ts | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts b/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts index 23d004c28f5..aac4e077e85 100644 --- a/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts +++ b/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts @@ -260,6 +260,7 @@ export default class CurlyComponentManager beginTrackFrame(); let props = processComponentArgs(capturedArgs); + props[ARGS] = capturedArgs; let argsTag = endTrackFrame(); // Alias `id` argument to `elementId` property on the component instance. diff --git a/packages/@ember/-internals/glimmer/lib/utils/process-args.ts b/packages/@ember/-internals/glimmer/lib/utils/process-args.ts index 51e79131dc5..27f0fc29956 100644 --- a/packages/@ember/-internals/glimmer/lib/utils/process-args.ts +++ b/packages/@ember/-internals/glimmer/lib/utils/process-args.ts @@ -3,7 +3,6 @@ import type { CapturedNamedArguments } from '@glimmer/interfaces'; import type { Reference } from '@glimmer/reference'; import { isUpdatableRef, updateRef, valueForRef } from '@glimmer/reference'; import { assert } from '@ember/debug'; -import { ARGS } from '../component-managers/curly'; import { ACTIONS } from '../helpers/action'; // ComponentArgs takes EvaluatedNamedArgs and converts them into the @@ -13,8 +12,6 @@ export function processComponentArgs(namedArgs: CapturedNamedArguments) { let attrs = Object.create(null); let props = Object.create(null); - props[ARGS] = namedArgs; - for (let name in namedArgs) { let ref = namedArgs[name]; assert('expected ref', ref); From cde70d2ab803f53cb174f5356dadb1ef8208635e Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Mon, 26 Aug 2024 15:44:52 -0400 Subject: [PATCH 09/17] fixing another cycle --- packages/@ember/-internals/glimmer/lib/utils/to-bool.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@ember/-internals/glimmer/lib/utils/to-bool.ts b/packages/@ember/-internals/glimmer/lib/utils/to-bool.ts index e3521eacadf..a17bdd505f7 100644 --- a/packages/@ember/-internals/glimmer/lib/utils/to-bool.ts +++ b/packages/@ember/-internals/glimmer/lib/utils/to-bool.ts @@ -1,4 +1,4 @@ -import { isHTMLSafe } from '@ember/-internals/glimmer'; +import { isHTMLSafe } from './string'; import { get, tagForProperty } from '@ember/-internals/metal'; import { isArray } from '@ember/array'; import { isProxy } from '@ember/-internals/utils'; From 4bd35632dfeef3c643558aca62fb7f6fc18043b3 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Mon, 26 Aug 2024 15:51:55 -0400 Subject: [PATCH 10/17] fixing another cycle --- packages/@ember/-internals/metal/lib/array.ts | 8 +------- packages/@ember/-internals/metal/lib/chain-tags.ts | 2 +- packages/@ember/-internals/metal/lib/object-at.ts | 9 +++++++++ 3 files changed, 11 insertions(+), 8 deletions(-) create mode 100644 packages/@ember/-internals/metal/lib/object-at.ts diff --git a/packages/@ember/-internals/metal/lib/array.ts b/packages/@ember/-internals/metal/lib/array.ts index 6049a15fa62..fa9f25ea28a 100644 --- a/packages/@ember/-internals/metal/lib/array.ts +++ b/packages/@ember/-internals/metal/lib/array.ts @@ -12,13 +12,7 @@ interface ObservedObject { _revalidate?: () => void; } -export function objectAt(array: T[] | EmberArray, index: number): T | undefined { - if (Array.isArray(array)) { - return array[index]; - } else { - return array.objectAt(index); - } -} +export { objectAt } from './object-at'; // Ideally, we'd use MutableArray.detect but for unknown reasons this causes // the node tests to fail strangely. diff --git a/packages/@ember/-internals/metal/lib/chain-tags.ts b/packages/@ember/-internals/metal/lib/chain-tags.ts index 4f7c13603dd..1aa1485da3a 100644 --- a/packages/@ember/-internals/metal/lib/chain-tags.ts +++ b/packages/@ember/-internals/metal/lib/chain-tags.ts @@ -10,7 +10,7 @@ import { updateTag, validateTag, } from '@glimmer/validator'; -import { objectAt } from './array'; +import { objectAt } from './object-at'; import { tagForProperty } from './tags'; export const CHAIN_PASS_THROUGH = new WeakSet(); diff --git a/packages/@ember/-internals/metal/lib/object-at.ts b/packages/@ember/-internals/metal/lib/object-at.ts new file mode 100644 index 00000000000..e85207863cb --- /dev/null +++ b/packages/@ember/-internals/metal/lib/object-at.ts @@ -0,0 +1,9 @@ +import type EmberArray from '@ember/array'; + +export function objectAt(array: T[] | EmberArray, index: number): T | undefined { + if (Array.isArray(array)) { + return array[index]; + } else { + return array.objectAt(index); + } +} From 02c93d43a1bf088fd00b812d3c069cee281da5ad Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Mon, 26 Aug 2024 17:36:48 -0400 Subject: [PATCH 11/17] break another cycle --- packages/@ember/-internals/metal/lib/observer.ts | 8 ++++---- packages/@ember/runloop/index.ts | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/@ember/-internals/metal/lib/observer.ts b/packages/@ember/-internals/metal/lib/observer.ts index 4cae45d4b62..f1f8035fdbe 100644 --- a/packages/@ember/-internals/metal/lib/observer.ts +++ b/packages/@ember/-internals/metal/lib/observer.ts @@ -1,6 +1,6 @@ import { ENV } from '@ember/-internals/environment'; import { peekMeta } from '@ember/-internals/meta'; -import { schedule } from '@ember/runloop'; +import type { schedule } from '@ember/runloop'; import { registerDestructor } from '@glimmer/destroyable'; import type { Tag } from '@glimmer/validator'; import { CURRENT_TAG, tagMetaFor, validateTag, valueForTag } from '@glimmer/validator'; @@ -187,7 +187,7 @@ export function revalidateObservers(target: object) { let lastKnownRevision = 0; -export function flushAsyncObservers(shouldSchedule = true) { +export function flushAsyncObservers(_schedule: typeof schedule | false) { let currentRevision = valueForTag(CURRENT_TAG); if (lastKnownRevision === currentRevision) { return; @@ -213,8 +213,8 @@ export function flushAsyncObservers(shouldSchedule = true) { } }; - if (shouldSchedule) { - schedule('actions', sendObserver); + if (_schedule) { + _schedule('actions', sendObserver); } else { sendObserver(); } diff --git a/packages/@ember/runloop/index.ts b/packages/@ember/runloop/index.ts index 4149c1c20e7..5cb41d34910 100644 --- a/packages/@ember/runloop/index.ts +++ b/packages/@ember/runloop/index.ts @@ -45,12 +45,12 @@ function onBegin(current: DeferredActionQueues) { function onEnd(_current: DeferredActionQueues, next: DeferredActionQueues) { currentRunLoop = next; - flushAsyncObservers(); + flushAsyncObservers(schedule); } function flush(queueName: string, next: () => void) { if (queueName === 'render' || queueName === _rsvpErrorQueue) { - flushAsyncObservers(); + flushAsyncObservers(schedule); } next(); From 49c0436ba7bee3bc1f1bc0a4ce55c9a1ba27f31c Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Mon, 26 Aug 2024 17:48:12 -0400 Subject: [PATCH 12/17] break another cycle --- .../-internals/glimmer/lib/templates/input.ts | 2 +- .../-internals/glimmer/lib/templates/link-to.ts | 2 +- .../-internals/glimmer/lib/templates/textarea.ts | 2 +- packages/@ember/modifier/index.ts | 15 +-------------- packages/@ember/modifier/on.ts | 16 ++++++++++++++++ packages/@ember/modifier/package.json | 3 ++- 6 files changed, 22 insertions(+), 18 deletions(-) create mode 100644 packages/@ember/modifier/on.ts diff --git a/packages/@ember/-internals/glimmer/lib/templates/input.ts b/packages/@ember/-internals/glimmer/lib/templates/input.ts index 295bb8d2a69..1d15a85275c 100644 --- a/packages/@ember/-internals/glimmer/lib/templates/input.ts +++ b/packages/@ember/-internals/glimmer/lib/templates/input.ts @@ -1,5 +1,5 @@ import { precompileTemplate } from '@ember/template-compilation'; -import { on } from '@ember/modifier'; +import { on } from '@ember/modifier/on'; export default precompileTemplate( ` {} - -// SAFETY: at the time of writing, the cast here is from `{}` to `OnModifier`, -// which makes it strictly safer to use outside this module because it is not -// usable as "any non-null item", which is what `{}` means, without loss of any -// information from the type itself. -export const on = glimmerOn as OnModifier; +export { on, type OnModifier } from './on'; // NOTE: this uses assignment to *require* that the `glimmerSetModifierManager` // is legally assignable to this type, i.e. that variance is properly upheld. diff --git a/packages/@ember/modifier/on.ts b/packages/@ember/modifier/on.ts new file mode 100644 index 00000000000..a90b73eb78e --- /dev/null +++ b/packages/@ember/modifier/on.ts @@ -0,0 +1,16 @@ +import { on as glimmerOn } from '@glimmer/runtime'; + +import type { Opaque } from '@ember/-internals/utility-types'; + +// In normal TypeScript, this modifier is essentially an opaque token that just +// needs to be importable. Declaring it with a unique interface like this, +// however, gives tools like Glint (that *do* have a richer notion of what it +// is) a place to install more detailed type information. +// eslint-disable-next-line @typescript-eslint/no-empty-interface +export interface OnModifier extends Opaque<'modifier:on'> {} + +// SAFETY: at the time of writing, the cast here is from `{}` to `OnModifier`, +// which makes it strictly safer to use outside this module because it is not +// usable as "any non-null item", which is what `{}` means, without loss of any +// information from the type itself. +export const on = glimmerOn as OnModifier; diff --git a/packages/@ember/modifier/package.json b/packages/@ember/modifier/package.json index 8207dcfbd39..292afe5c459 100644 --- a/packages/@ember/modifier/package.json +++ b/packages/@ember/modifier/package.json @@ -3,7 +3,8 @@ "private": true, "type": "module", "exports": { - ".": "./index.ts" + ".": "./index.ts", + "./on": "./on.ts" }, "dependencies": { "@ember/-internals": "workspace:*", From 1d2ec228f37252e3ed2f2613b999c4d9fb135500 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Tue, 27 Aug 2024 09:40:39 -0400 Subject: [PATCH 13/17] breaking another cycle --- .../-internals/glimmer/lib/components/link-to.ts | 12 ++++++------ packages/@ember/engine/index.ts | 2 +- packages/@ember/engine/instance.ts | 2 +- packages/@ember/engine/package.json | 3 ++- .../engine/{lib/engine-parent.ts => parent.ts} | 2 +- 5 files changed, 11 insertions(+), 10 deletions(-) rename packages/@ember/engine/{lib/engine-parent.ts => parent.ts} (94%) diff --git a/packages/@ember/-internals/glimmer/lib/components/link-to.ts b/packages/@ember/-internals/glimmer/lib/components/link-to.ts index f5110758a4e..4cba92603c9 100644 --- a/packages/@ember/-internals/glimmer/lib/components/link-to.ts +++ b/packages/@ember/-internals/glimmer/lib/components/link-to.ts @@ -2,8 +2,8 @@ import type Route from '@ember/routing/route'; import type { RouterState, RoutingService } from '@ember/routing/-internals'; import { isSimpleClick } from '@ember/-internals/views'; import { assert, debugFreeze, inspect, warn } from '@ember/debug'; -import { getEngineParent } from '@ember/engine'; -import EngineInstance from '@ember/engine/instance'; +import { getEngineParent } from '@ember/engine/parent'; +import type EngineInstance from '@ember/engine/instance'; import { flaggedInstrument } from '@ember/instrumentation'; import { action } from '@ember/object'; import { service } from '@ember/service'; @@ -494,13 +494,13 @@ class _LinkTo extends InternalComponent { } private get isEngine(): boolean { - let owner = this.owner; - return owner instanceof EngineInstance && getEngineParent(owner) !== undefined; + let owner = this.owner as EngineInstance; + return getEngineParent(owner) !== undefined; } private get engineMountPoint(): string | undefined { - let owner = this.owner; - return owner instanceof EngineInstance ? owner.mountPoint : undefined; + let owner = this.owner as EngineInstance; + return owner.mountPoint; } private classFor(state: 'active' | 'loading' | 'disabled'): string { diff --git a/packages/@ember/engine/index.ts b/packages/@ember/engine/index.ts index 27bfd5ba94b..25b756b9deb 100644 --- a/packages/@ember/engine/index.ts +++ b/packages/@ember/engine/index.ts @@ -1,4 +1,4 @@ -export { getEngineParent, setEngineParent } from './lib/engine-parent'; +export { getEngineParent, setEngineParent } from './parent'; import { canInvoke } from '@ember/-internals/utils'; import Controller from '@ember/controller'; diff --git a/packages/@ember/engine/instance.ts b/packages/@ember/engine/instance.ts index 1565b8eca83..097b5c6ca6e 100644 --- a/packages/@ember/engine/instance.ts +++ b/packages/@ember/engine/instance.ts @@ -7,7 +7,7 @@ import { RSVP } from '@ember/-internals/runtime'; import { assert } from '@ember/debug'; import { Registry, privatize as P } from '@ember/-internals/container'; import { guidFor } from '@ember/-internals/utils'; -import { ENGINE_PARENT, getEngineParent, setEngineParent } from './lib/engine-parent'; +import { ENGINE_PARENT, getEngineParent, setEngineParent } from './parent'; import { ContainerProxyMixin, RegistryProxyMixin } from '@ember/-internals/runtime'; import type { InternalOwner } from '@ember/-internals/owner'; import type Owner from '@ember/-internals/owner'; diff --git a/packages/@ember/engine/package.json b/packages/@ember/engine/package.json index 3d8c7d16171..394a2008320 100644 --- a/packages/@ember/engine/package.json +++ b/packages/@ember/engine/package.json @@ -4,7 +4,8 @@ "type": "module", "exports": { ".": "./index.ts", - "./instance": "./instance.ts" + "./instance": "./instance.ts", + "./parent": "./parent.ts" }, "dependencies": { "@ember/-internals": "workspace:*", diff --git a/packages/@ember/engine/lib/engine-parent.ts b/packages/@ember/engine/parent.ts similarity index 94% rename from packages/@ember/engine/lib/engine-parent.ts rename to packages/@ember/engine/parent.ts index cabd777b07b..6c988263ef5 100644 --- a/packages/@ember/engine/lib/engine-parent.ts +++ b/packages/@ember/engine/parent.ts @@ -1,7 +1,7 @@ /** @module @ember/engine */ -import type EngineInstance from '../instance'; +import type EngineInstance from './instance'; export const ENGINE_PARENT = Symbol('ENGINE_PARENT'); From 821f02eaa7eae422d3228fe1bed882f75f7592ae Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Tue, 27 Aug 2024 09:42:02 -0400 Subject: [PATCH 14/17] enforce no more cycles --- rollup.config.mjs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/rollup.config.mjs b/rollup.config.mjs index 901b2a3ad7e..19bf5cebc94 100644 --- a/rollup.config.mjs +++ b/rollup.config.mjs @@ -58,10 +58,7 @@ function esmConfig() { process.stderr.write( `Circular dependency:\n${log.ids.map((id) => ' ' + id).join('\n')}\n` ); - break; - case 'CYCLIC_CROSS_CHUNK_REEXPORT': - case 'EMPTY_BUNDLE': - break; + throw new Error(`Circular dependencies are forbidden`); default: handler(level, log); } From 70da3fbad8827f9d0a042ccce59fc7f48490f371 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Tue, 27 Aug 2024 09:45:18 -0400 Subject: [PATCH 15/17] silence empty bundles warning --- rollup.config.mjs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/rollup.config.mjs b/rollup.config.mjs index 19bf5cebc94..40588430934 100644 --- a/rollup.config.mjs +++ b/rollup.config.mjs @@ -59,6 +59,12 @@ function esmConfig() { `Circular dependency:\n${log.ids.map((id) => ' ' + id).join('\n')}\n` ); throw new Error(`Circular dependencies are forbidden`); + case 'EMPTY_BUNDLE': + // Some of our entrypoints are type-only and result in empty bundles. + // We prune the actual empty files elsewhere in this config (see + // pruneEmptyBundles). This silences the warning from rollup about + // them. + return; default: handler(level, log); } From 6232f4a4551326cdf33248f348ac018085d18662 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Tue, 27 Aug 2024 09:49:31 -0400 Subject: [PATCH 16/17] keep lib/engine-parent for legacy bundle --- packages/@ember/engine/lib/engine-parent.ts | 32 ++++++++++++++++++++ packages/@ember/engine/parent.ts | 33 +-------------------- 2 files changed, 33 insertions(+), 32 deletions(-) create mode 100644 packages/@ember/engine/lib/engine-parent.ts diff --git a/packages/@ember/engine/lib/engine-parent.ts b/packages/@ember/engine/lib/engine-parent.ts new file mode 100644 index 00000000000..6c988263ef5 --- /dev/null +++ b/packages/@ember/engine/lib/engine-parent.ts @@ -0,0 +1,32 @@ +/** +@module @ember/engine +*/ +import type EngineInstance from './instance'; + +export const ENGINE_PARENT = Symbol('ENGINE_PARENT'); + +/** + `getEngineParent` retrieves an engine instance's parent instance. + + @method getEngineParent + @param {EngineInstance} engine An engine instance. + @return {EngineInstance} The parent engine instance. + @for @ember/engine + @static + @private +*/ +export function getEngineParent(engine: EngineInstance): EngineInstance | undefined { + return engine[ENGINE_PARENT]; +} + +/** + `setEngineParent` sets an engine instance's parent instance. + + @method setEngineParent + @param {EngineInstance} engine An engine instance. + @param {EngineInstance} parent The parent engine instance. + @private +*/ +export function setEngineParent(engine: EngineInstance, parent: EngineInstance): void { + engine[ENGINE_PARENT] = parent; +} diff --git a/packages/@ember/engine/parent.ts b/packages/@ember/engine/parent.ts index 6c988263ef5..61bd5a7eeb4 100644 --- a/packages/@ember/engine/parent.ts +++ b/packages/@ember/engine/parent.ts @@ -1,32 +1 @@ -/** -@module @ember/engine -*/ -import type EngineInstance from './instance'; - -export const ENGINE_PARENT = Symbol('ENGINE_PARENT'); - -/** - `getEngineParent` retrieves an engine instance's parent instance. - - @method getEngineParent - @param {EngineInstance} engine An engine instance. - @return {EngineInstance} The parent engine instance. - @for @ember/engine - @static - @private -*/ -export function getEngineParent(engine: EngineInstance): EngineInstance | undefined { - return engine[ENGINE_PARENT]; -} - -/** - `setEngineParent` sets an engine instance's parent instance. - - @method setEngineParent - @param {EngineInstance} engine An engine instance. - @param {EngineInstance} parent The parent engine instance. - @private -*/ -export function setEngineParent(engine: EngineInstance, parent: EngineInstance): void { - engine[ENGINE_PARENT] = parent; -} +export * from './lib/engine-parent'; From 0945aaca16660bafbe65c0eaaa0a8e425fb08db4 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Tue, 27 Aug 2024 09:49:41 -0400 Subject: [PATCH 17/17] share rollup onLog across all builds --- packages/@ember/engine/lib/engine-parent.ts | 2 +- rollup.config.mjs | 45 +++++++++++---------- 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/packages/@ember/engine/lib/engine-parent.ts b/packages/@ember/engine/lib/engine-parent.ts index 6c988263ef5..cabd777b07b 100644 --- a/packages/@ember/engine/lib/engine-parent.ts +++ b/packages/@ember/engine/lib/engine-parent.ts @@ -1,7 +1,7 @@ /** @module @ember/engine */ -import type EngineInstance from './instance'; +import type EngineInstance from '../instance'; export const ENGINE_PARENT = Symbol('ENGINE_PARENT'); diff --git a/rollup.config.mjs b/rollup.config.mjs index 40588430934..e7b775fee36 100644 --- a/rollup.config.mjs +++ b/rollup.config.mjs @@ -48,27 +48,7 @@ function esmConfig() { ]; return { - onLog(level, log, handler) { - switch (log.code) { - case 'CIRCULAR_DEPENDENCY': - if (log.ids.some((id) => id.includes('node_modules/rsvp/lib/rsvp'))) { - // rsvp has some internal cycles but they don't bother us - return; - } - process.stderr.write( - `Circular dependency:\n${log.ids.map((id) => ' ' + id).join('\n')}\n` - ); - throw new Error(`Circular dependencies are forbidden`); - case 'EMPTY_BUNDLE': - // Some of our entrypoints are type-only and result in empty bundles. - // We prune the actual empty files elsewhere in this config (see - // pruneEmptyBundles). This silences the warning from rollup about - // them. - return; - default: - handler(level, log); - } - }, + onLog: handleRollupWarnings, input: { ...renameEntrypoints(exposedDependencies(), (name) => join('packages', name, 'index')), ...renameEntrypoints(packages(), (name) => join('packages', name)), @@ -126,6 +106,7 @@ function legacyBundleConfig(input, output, { isDeveloping, isExternal }) { interop: 'esModule', }, + onLog: handleRollupWarnings, plugins: [ amdDefineSupport(), ...(isDeveloping ? [concatenateAMDEntrypoints()] : []), @@ -516,3 +497,25 @@ function pruneEmptyBundles() { }, }; } + +function handleRollupWarnings(level, log, handler) { + switch (log.code) { + case 'CIRCULAR_DEPENDENCY': + if (log.ids.some((id) => id.includes('node_modules/rsvp/lib/rsvp'))) { + // rsvp has some internal cycles but they don't bother us + return; + } + process.stderr.write( + `Circular dependency:\n${log.ids.map((id) => ' ' + id).join('\n')}\n` + ); + throw new Error(`Circular dependencies are forbidden`); + case 'EMPTY_BUNDLE': + // Some of our entrypoints are type-only and result in empty bundles. + // We prune the actual empty files elsewhere in this config (see + // pruneEmptyBundles). This silences the warning from rollup about + // them. + return; + default: + handler(level, log); + } +}