Skip to content

Commit

Permalink
Upgrade VM @glimmer/* packages from 0.84.3 to 0.85.13 (emberjs#20561)
Browse files Browse the repository at this point in the history
* Bump vm packages

Upgrade VM @glimmer/* packages

Also, @glimmer/vm needed to be added to the root package.json so non-declared dependencies (such as @ember/-internals) may be provided access to @glimmer/vm

Upgrade glimmer-vm again

* Address removal of Option from '@glimmer/interfaces' by internalizing the type to @ember/-internals/utility-types

* Remove import of WeakSet polyfill as it's shipped everywhere now, and glimmer doesn't provide the polyfill anymore

* ComponentDefinition's capabilities property is now of type CapabilityMask, instead of InternalComponentCapability

* @glimmer/validator's setTrackingTransactionEnv is now only accessible via the 'debug' export

* Cast away some Reference<unknown> types to Reference<more specific> and note that GlimmerVM needs further changes

* CurriedType is now in @glimmer/vm, because it's a value, not just a type/interface

* lint:fix

* Update usage of glimmer/babel-vm-plugins

* programCompilationContext needs a third parameter

* Use correct CurriedType value

* 'fix' type of a ref in outlet

* Force rollup v4 so we have real errors that we can understand as humans.

This revealed that we don't need @babel/plugin-transform-block-scoping

* We re-implement babel features for private fields, since glimmer-vm now
ships them untranspiled

* Internal type checking now passes. the debug render tree args now requires an 'errors' object so that errors can be reported back to the user if something goes wrong

* Add disable_local_debug to qunit, as we have a bug we need to fix, but it was likely pre-existing

Add disable_local_debug to bin/run-tests

* Address incorrect type for stateFor

* Update comment to be correct and note an optimization

* Fix the type of the refs for closureAction

Changing AnyFn to Function in places up until the runloop public API

* Rename Option to Nullable to align with glimmer-vm terminology

* lockfile maintenance

* Revert "Internal type checking now passes. the debug render tree args now requires an 'errors' object so that errors can be reported back to the user if something goes wrong"

This reverts commit 0092ad7.

* Push Reference<unknown> into makeClosureAction

* Revert "Add disable_local_debug to qunit, as we have a bug we need to fix, but it was likely pre-existing"

This reverts commit 1856b8c.

* Apply suggestions from code review

Co-authored-by: Godfrey Chan <godfreykfc@gmail.com>

* lint:fix

* Update packages/@ember/-internals/glimmer/lib/helpers/action.ts

---------

Co-authored-by: Godfrey Chan <godfreykfc@gmail.com>
  • Loading branch information
NullVoxPopuli and chancancode committed Nov 15, 2023
1 parent f5572ac commit 32c669e
Show file tree
Hide file tree
Showing 56 changed files with 587 additions and 438 deletions.
5 changes: 5 additions & 0 deletions .npmrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# super strict mode
auto-install-peers=false
resolve-peers-from-workspace-root=false


6 changes: 1 addition & 5 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const buildDebugMacroPlugin = require('./build-debug-macro-plugin');
const buildStripClassCallcheckPlugin = require('./build-strip-class-callcheck-plugin');
const injectBabelHelpers = require('./transforms/inject-babel-helpers').injectBabelHelpers;
const debugTree = require('broccoli-debug').buildDebugCallback('ember-source:addon');
const vmBabelPlugins = require('@glimmer/vm-babel-plugins');
const { default: vmBabelPlugins } = require('@glimmer/vm-babel-plugins');
const Overrides = require('./overrides');
const SilentError = require('silent-error');
const SupportedBrowsers = require('./browsers');
Expand Down Expand Up @@ -156,10 +156,6 @@ module.exports = {
babelHelperPlugin,
buildDebugMacroPlugin(!isProduction),
...vmBabelPlugins({ isDebug: !isProduction }),
[
require.resolve('@babel/plugin-transform-block-scoping'),
{ throwIfClosureRequired: true },
],
],
}),
};
Expand Down
39 changes: 22 additions & 17 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,25 +55,25 @@
},
"dependencies": {
"@babel/helper-module-imports": "^7.16.7",
"@babel/plugin-transform-block-scoping": "^7.22.5",
"@ember/edition-utils": "^1.2.0",
"@glimmer/compiler": "0.84.3",
"@glimmer/compiler": "0.85.13",
"@glimmer/component": "^1.1.2",
"@glimmer/destroyable": "0.84.3",
"@glimmer/destroyable": "0.85.13",
"@glimmer/env": "^0.1.7",
"@glimmer/global-context": "0.84.3",
"@glimmer/interfaces": "0.84.3",
"@glimmer/manager": "0.84.3",
"@glimmer/node": "0.84.3",
"@glimmer/opcode-compiler": "0.84.3",
"@glimmer/owner": "0.84.3",
"@glimmer/program": "0.84.3",
"@glimmer/reference": "0.84.3",
"@glimmer/runtime": "0.84.3",
"@glimmer/syntax": "0.84.3",
"@glimmer/util": "0.84.3",
"@glimmer/validator": "0.84.3",
"@glimmer/vm-babel-plugins": "0.84.3",
"@glimmer/global-context": "0.85.13",
"@glimmer/interfaces": "0.85.13",
"@glimmer/manager": "0.85.13",
"@glimmer/node": "0.85.13",
"@glimmer/opcode-compiler": "0.85.13",
"@glimmer/owner": "0.85.13",
"@glimmer/program": "0.85.13",
"@glimmer/reference": "0.85.13",
"@glimmer/runtime": "0.85.13",
"@glimmer/syntax": "0.85.13",
"@glimmer/util": "0.85.13",
"@glimmer/validator": "0.85.13",
"@glimmer/vm": "0.85.13",
"@glimmer/vm-babel-plugins": "0.85.13",
"@simple-dom/interface": "^1.4.0",
"babel-plugin-debug-macros": "^0.3.4",
"babel-plugin-filter-imports": "^4.0.0",
Expand Down Expand Up @@ -173,6 +173,11 @@
"resolutions": {
"socket.io": "^4.7.0"
},
"pnpm": {
"overrides": {
"rollup": "^4.2.0"
}
},
"peerDependencies": {
"@glimmer/component": "^1.1.2"
},
Expand All @@ -194,6 +199,6 @@
},
"volta": {
"node": "16.20.0",
"pnpm": "8.7.6"
"pnpm": "8.10.0"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
} from '@ember/-internals/owner';
import { enumerableSymbol, guidFor } from '@ember/-internals/utils';
import { addChildView, setElementView, setViewElement } from '@ember/-internals/views';
import type { Nullable } from '@ember/-internals/utility-types';
import { assert, debugFreeze } from '@ember/debug';
import { _instrumentStart } from '@ember/instrumentation';
import { DEBUG } from '@glimmer/env';
Expand All @@ -17,7 +18,6 @@ import type {
ElementOperations,
Environment,
InternalComponentCapabilities,
Option,
PreparedArguments,
TemplateFactory,
VMArguments,
Expand Down Expand Up @@ -154,7 +154,7 @@ export default class CurlyComponentManager
return this.templateFor(bucket.component);
}

getTagName(state: ComponentStateBucket): Option<string> {
getTagName(state: ComponentStateBucket): Nullable<string> {
let { component, hasWrappedElement } = state;

if (!hasWrappedElement) {
Expand All @@ -168,7 +168,7 @@ export default class CurlyComponentManager
return CURLY_CAPABILITIES;
}

prepareArgs(ComponentClass: ComponentFactory, args: VMArguments): Option<PreparedArguments> {
prepareArgs(ComponentClass: ComponentFactory, args: VMArguments): Nullable<PreparedArguments> {
if (args.named.has('__ARGS__')) {
assert(
'[BUG] cannot pass both __ARGS__ and positional arguments',
Expand Down Expand Up @@ -467,7 +467,7 @@ export default class CurlyComponentManager
}
}

getDestroyable(bucket: ComponentStateBucket): Option<Destroyable> {
getDestroyable(bucket: ComponentStateBucket): Nullable<Destroyable> {
return bucket;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ import type {
Destroyable,
Environment,
InternalComponentCapabilities,
Option,
TemplateFactory,
VMArguments,
WithCreateInstance,
WithCustomDebugRenderTree,
WithDynamicLayout,
WithSubOwner,
} from '@glimmer/interfaces';
import type { Nullable } from '@ember/-internals/utility-types';
import { capabilityFlagsFrom } from '@glimmer/manager';
import type { Reference } from '@glimmer/reference';
import { createConstRef, valueForRef } from '@glimmer/reference';
Expand Down Expand Up @@ -149,7 +149,7 @@ class MountManager
return self;
}

getDestroyable(bucket: EngineState): Option<Destroyable> {
getDestroyable(bucket: EngineState): Nullable<Destroyable> {
return bucket.engine;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,18 @@ import type {
CapturedArguments,
CompilableProgram,
ComponentDefinition,
CapabilityMask,
CustomRenderNode,
Destroyable,
Environment,
InternalComponentCapabilities,
InternalComponentCapability,
Option,
Template,
VMArguments,
WithCreateInstance,
WithCustomDebugRenderTree,
WithDynamicTagName,
} from '@glimmer/interfaces';
import type { Nullable } from '@ember/-internals/utility-types';
import { capabilityFlagsFrom } from '@glimmer/manager';
import type { Reference } from '@glimmer/reference';
import { createConstRef, valueForRef } from '@glimmer/reference';
Expand Down Expand Up @@ -179,7 +179,7 @@ class OutletComponentManager

didUpdateLayout() {}

getDestroyable(): Option<Destroyable> {
getDestroyable(): Nullable<Destroyable> {
return null;
}
}
Expand All @@ -195,7 +195,7 @@ export class OutletComponentDefinition

public resolvedName: string;
public compilable: CompilableProgram;
public capabilities: InternalComponentCapability;
public capabilities: CapabilityMask;

constructor(
public state: OutletDefinitionState,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ import type {
ComponentDefinition,
Environment,
InternalComponentCapabilities,
Option,
Owner,
VMArguments,
} from '@glimmer/interfaces';
import type { Nullable } from '@ember/-internals/utility-types';
import { capabilityFlagsFrom } from '@glimmer/manager';
import { CONSTANT_TAG, consumeTag } from '@glimmer/validator';
import type Component from '../component';
Expand All @@ -32,7 +32,7 @@ class RootComponentManager extends CurlyComponentManager {
create(
_owner: Owner,
_state: unknown,
_args: Option<VMArguments>,
_args: Nullable<VMArguments>,
{ isInteractive }: Environment,
dynamicScope: DynamicScope
) {
Expand Down
9 changes: 5 additions & 4 deletions packages/@ember/-internals/glimmer/lib/components/link-to.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import { flaggedInstrument } from '@ember/instrumentation';
import { action } from '@ember/object';
import { service } from '@ember/service';
import { DEBUG } from '@glimmer/env';
import type { Maybe, Option } from '@glimmer/interfaces';
import type { Maybe } from '@glimmer/interfaces';
import type { Nullable } from '@ember/-internals/utility-types';
import { consumeTag, createCache, getValue, tagFor, untrack } from '@glimmer/validator';
import type { Transition } from 'router_js';
import LinkToTemplate from '../templates/link-to';
Expand All @@ -31,7 +32,7 @@ function isPresent<T>(value: Maybe<T>): value is T {

interface QueryParams {
isQueryParams: true;
values: Option<{}>;
values: Nullable<{}>;
}

function isQueryParams(value: unknown): value is QueryParams {
Expand Down Expand Up @@ -473,7 +474,7 @@ class _LinkTo extends InternalComponent {
return this.isActiveForState(this.routing.currentState as Maybe<RouterState>);
}

private get willBeActive(): Option<boolean> {
private get willBeActive(): Nullable<boolean> {
let current = this.routing.currentState;
let target = this.routing.targetState;

Expand Down Expand Up @@ -585,7 +586,7 @@ class _LinkTo extends InternalComponent {

let { prototype } = _LinkTo;

let descriptorFor = (target: object, property: string): Option<PropertyDescriptor> => {
let descriptorFor = (target: object, property: string): Nullable<PropertyDescriptor> => {
if (target) {
return (
Object.getOwnPropertyDescriptor(target, property) ||
Expand Down
4 changes: 2 additions & 2 deletions packages/@ember/-internals/glimmer/lib/environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { schedule, _backburner } from '@ember/runloop';
import { DEBUG } from '@glimmer/env';
import setGlobalContext from '@glimmer/global-context';
import type { EnvironmentDelegate } from '@glimmer/runtime';
import { setTrackingTransactionEnv } from '@glimmer/validator';
import { debug } from '@glimmer/validator';
import toIterator from './utils/iterator';
import { isHTMLSafe } from './utils/string';
import toBool from './utils/to-bool';
Expand Down Expand Up @@ -92,7 +92,7 @@ setGlobalContext({
});

if (DEBUG) {
setTrackingTransactionEnv?.({
debug?.setTrackingTransactionEnv?.({
debugMessage(obj, keyName) {
let dirtyString = keyName
? `\`${keyName}\` on \`${getDebugName?.(obj)}\``
Expand Down
45 changes: 25 additions & 20 deletions packages/@ember/-internals/glimmer/lib/helpers/action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,9 @@ import { DEBUG } from '@glimmer/env';
import type { CapturedArguments } from '@glimmer/interfaces';
import type { Reference } from '@glimmer/reference';
import { createUnboundRef, isInvokableRef, updateRef, valueForRef } from '@glimmer/reference';
import { _WeakSet } from '@glimmer/util';
import { internalHelper } from './internal-helper';

export const ACTIONS = new _WeakSet();
export const ACTIONS = new WeakSet();

/**
The `{{action}}` helper provides a way to pass triggers for behavior (usually
Expand Down Expand Up @@ -299,7 +298,12 @@ export default internalHelper((args: CapturedArguments): Reference<Function> =>
} else {
fn = makeDynamicClosureAction(
valueForRef(context) as object,
// SAFETY: glimmer-vm should expose narrowing utilities for references
// as is, `target` is still `Reference<unknown>`.
// however, we never even tried to narrow `target`, so this is potentially risky code.
target!,
// SAFETY: glimmer-vm should expose narrowing utilities for references
// as is, `action` is still `Reference<unknown>`
action,
processArgs,
debugKey
Expand Down Expand Up @@ -349,57 +353,58 @@ function makeArgsProcessor(valuePathRef: Reference | false, actionArgsRef: Refer

function makeDynamicClosureAction(
context: object,
targetRef: Reference<MaybeActionHandler>,
actionRef: Reference<string | AnyFn>,
targetRef: Reference<unknown>,
actionRef: Reference<unknown>,
processArgs: (args: unknown[]) => unknown[],
debugKey: string
) {
const action = valueForRef(actionRef);

// We don't allow undefined/null values, so this creates a throw-away action to trigger the assertions
if (DEBUG) {
makeClosureAction(
context,
valueForRef(targetRef),
valueForRef(actionRef),
processArgs,
debugKey
);
makeClosureAction(context, valueForRef(targetRef), action, processArgs, debugKey);
}

return (...args: any[]) => {
return makeClosureAction(
context,
valueForRef(targetRef),
valueForRef(actionRef),
action,
processArgs,
debugKey
)(...args);
};
}

interface MaybeActionHandler {
actions?: Record<string, AnyFn>;
actions?: Record<string, Function>;
}

function makeClosureAction(
context: object,
target: MaybeActionHandler,
action: string | AnyFn,
target: unknown,
action: unknown | null | undefined | string | Function,
processArgs: (args: unknown[]) => unknown[],
debugKey: string
) {
let self: object;
let fn: AnyFn;
let fn: Function;

assert(
`Action passed is null or undefined in (action) from ${target}.`,
action !== undefined && action !== null
);

if (typeof action === 'string') {
assert('target must be an object', target !== null && typeof target === 'object');
self = target;
fn = (target.actions && target.actions[action as string])!;

assert(`An action named '${action}' was not found in ${target}`, Boolean(fn));
let value = (target as { actions?: Record<string, unknown> }).actions?.[action];
assert(`An action named '${action}' was not found in ${target}`, Boolean(value));
assert(
`An action named '${action}' was found in ${target}, but is not a function`,
typeof value === 'function'
);
fn = value;
} else if (typeof action === 'function') {
self = context;
fn = action;
Expand All @@ -417,7 +422,7 @@ function makeClosureAction(
return (...args: any[]) => {
let payload = { target: self, args, label: '@glimmer/closure-action' };
return flaggedInstrument('interaction.ember-action', payload, () => {
return join(self, fn, ...processArgs(args));
return join(self, fn as AnyFn, ...processArgs(args));
});
};
}
Expand Down
7 changes: 6 additions & 1 deletion packages/@ember/-internals/glimmer/lib/helpers/unique-id.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,12 @@ import { createConstRef } from '@glimmer/reference';
import { internalHelper } from './internal-helper';

export default internalHelper((): Reference<string> => {
return createConstRef(uniqueId(), 'unique-id');
// SAFETY: glimmer-vm should change the signature of createUnboundRef to use a generic
// so that the type param to `Reference<?>` can infer from the first argument.
//
// NOTE: constRef is an optimization so we don't let the VM create extra wrappers,
// tracking frames, etc.
return createConstRef(uniqueId(), 'unique-id') as Reference<string>;
});

// From https://gist.github.com/selfish/fef2c0ba6cdfe07af76e64cecd74888b
Expand Down

0 comments on commit 32c669e

Please sign in to comment.