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

[FEATURE strict-mode] Update VM for Strict Mode #19302

Merged
merged 1 commit into from
Dec 11, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
24 changes: 12 additions & 12 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,19 +74,19 @@
},
"devDependencies": {
"@babel/preset-env": "^7.9.5",
"@glimmer/compiler": "0.68.1",
"@glimmer/compiler": "0.69.3",
"@glimmer/env": "^0.1.7",
"@glimmer/global-context": "0.68.1",
"@glimmer/interfaces": "0.68.1",
"@glimmer/manager": "0.68.1",
"@glimmer/destroyable": "0.68.1",
"@glimmer/owner": "0.68.1",
"@glimmer/node": "0.68.1",
"@glimmer/opcode-compiler": "0.68.1",
"@glimmer/program": "0.68.1",
"@glimmer/reference": "0.68.1",
"@glimmer/runtime": "0.68.1",
"@glimmer/validator": "0.68.1",
"@glimmer/global-context": "0.69.3",
"@glimmer/interfaces": "0.69.3",
"@glimmer/manager": "0.69.3",
"@glimmer/destroyable": "0.69.3",
"@glimmer/owner": "0.69.3",
"@glimmer/node": "0.69.3",
"@glimmer/opcode-compiler": "0.69.3",
"@glimmer/program": "0.69.3",
"@glimmer/reference": "0.69.3",
"@glimmer/runtime": "0.69.3",
"@glimmer/validator": "0.69.3",
"@simple-dom/document": "^1.4.0",
"@types/qunit": "^2.9.1",
"@types/rsvp": "^4.0.3",
Expand Down
9 changes: 8 additions & 1 deletion packages/@ember/-internals/glimmer/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,7 @@ export { default as Checkbox } from './lib/components/checkbox';
export { default as TextField } from './lib/components/text-field';
export { default as TextArea } from './lib/components/textarea';
export { default as LinkComponent } from './lib/components/link-to';
export { InputComponent as Input } from './lib/components/input';
export { default as Component } from './lib/component';
export { default as Helper, helper } from './lib/helper';
export { SafeString, escapeExpression, htmlSafe, isHTMLSafe } from './lib/utils/string';
Expand All @@ -387,12 +388,18 @@ export {
export { setupEngineRegistry, setupApplicationRegistry } from './lib/setup-registry';
export { DOMChanges, NodeDOMTreeConstruction, DOMTreeConstruction } from './lib/dom';

export { default as array } from './lib/helpers/array';
export { default as hash } from './lib/helpers/hash';
export { default as concat } from './lib/helpers/concat';
export { default as get } from './lib/helpers/get';
export { default as fn } from './lib/helpers/fn';
export { default as on } from './lib/modifiers/on';

// needed for test
// TODO just test these through public API
// a lot of these are testing how a problem was solved
// rather than the problem was solved
export { INVOKE } from './lib/helpers/action';
export { default as on } from './lib/modifiers/on';
export { default as OutletView } from './lib/views/outlet';
export { OutletState } from './lib/utils/outlet';
export { setComponentManager } from './lib/utils/managers';
Expand Down
15 changes: 12 additions & 3 deletions packages/@ember/-internals/glimmer/lib/component-managers/curly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,10 @@ type ComponentFactory = Factory<
positionalParams: string | string[] | undefined | null;
name: string;
}
>;
> & {
name: string;
positionalParams: string | string[] | undefined | null;
};

export default class CurlyComponentManager
implements
Expand Down Expand Up @@ -188,7 +191,7 @@ export default class CurlyComponentManager
return prepared;
}

const { positionalParams } = ComponentClass.class!;
const { positionalParams } = ComponentClass.class ?? ComponentClass;

// early exits
if (
Expand Down Expand Up @@ -355,7 +358,9 @@ export default class CurlyComponentManager
}

getDebugName(definition: ComponentFactory): string {
return definition.fullName || definition.normalizedName || definition.class!.name;
return (
definition.fullName || definition.normalizedName || definition.class?.name || definition.name
);
}

getSelf({ rootRef }: ComponentStateBucket): Reference {
Expand Down Expand Up @@ -551,3 +556,7 @@ export const CURLY_CAPABILITIES: InternalComponentCapabilities = {
};

export const CURLY_COMPONENT_MANAGER = new CurlyComponentManager();

export function isCurlyManager(manager: object): boolean {
return manager === CURLY_COMPONENT_MANAGER;
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,26 @@ export default class InternalManager
implements
InternalComponentManager<InternalComponentState, EmberInternalComponentConstructor>,
WithCreateInstance {
static for(name: string): (owner: Owner) => InternalManager {
return (owner: Owner) => new InternalManager(owner, name);
static for(
definition: EmberInternalComponentConstructor,
name: string
): (owner: Owner) => InternalManager {
return (owner: Owner) => new InternalManager(owner, definition, name);
}

constructor(private owner: Owner, private name: string) {}
constructor(
private owner: Owner,
private ComponentClass: EmberInternalComponentConstructor,
private name: string
) {}

getCapabilities(): InternalComponentCapabilities {
return CAPABILITIES;
}

create(
env: Environment,
ComponentClass: EmberInternalComponentConstructor,
_definition: unknown,
args: VMArguments,
_dynamicScope: DynamicScope,
caller: Reference
Expand All @@ -64,7 +71,8 @@ export default class InternalManager
args.positional.length === 0
);

let instance = new ComponentClass(this.owner, args.named.capture(), valueForRef(caller));
let { ComponentClass, owner } = this;
let instance = new ComponentClass(owner, args.named.capture(), valueForRef(caller));

let state = { env, instance };

Expand Down
16 changes: 13 additions & 3 deletions packages/@ember/-internals/glimmer/lib/components/input.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
/**
@module @ember/component
*/
import { setInternalComponentManager } from '@glimmer/manager';
import { assert } from '@ember/debug';
import { setComponentTemplate, setInternalComponentManager } from '@glimmer/manager';
import InternalManager from '../component-managers/internal';
import InputTemplate from '../templates/input';
import InternalComponent from './internal';

/**
See [Ember.Templates.components.Input](/ember/release/classes/Ember.Templates.components/methods/Input?anchor=Input).

Expand Down Expand Up @@ -117,6 +118,15 @@ export default class Input extends InternalComponent {
}
}

setInternalComponentManager(InternalManager.for('input'), Input);
// Use an opaque handle so implementation details are
export const InputComponent = {
// Factory interface
create(): never {
throw assert('Use constructor instead of create');
Copy link
Member

Choose a reason for hiding this comment

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

Throw assert seems odd? (Non blocking as it was already like this)

},
};

setInternalComponentManager(InternalManager.for(Input, 'input'), InputComponent);
setComponentTemplate(InputTemplate, InputComponent);

Input.toString = () => '@ember/component/input';
6 changes: 0 additions & 6 deletions packages/@ember/-internals/glimmer/lib/components/internal.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,8 @@
import { Owner, setOwner } from '@ember/-internals/owner';
import { guidFor } from '@ember/-internals/utils';
import { assert } from '@ember/debug';
import { Reference, valueForRef } from '@glimmer/reference';

export default class InternalComponent {
// Factory interface
static create(): never {
throw assert('Use constructor instead of create');
}

static get class(): typeof InternalComponent {
return this;
}
Expand Down
51 changes: 35 additions & 16 deletions packages/@ember/-internals/glimmer/lib/resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import {
templateOnlyComponent,
} from '@glimmer/runtime';
import { _WeakSet } from '@glimmer/util';
import { CURLY_COMPONENT_MANAGER } from './component-managers/curly';
import { isCurlyManager } from './component-managers/curly';
import {
CLASSIC_HELPER_MANAGER_FACTORY,
HelperFactory,
Expand Down Expand Up @@ -177,20 +177,15 @@ if (PARTIALS) {
};
}

const BUILTINS_HELPERS = {
if: inlineIf,
const BUILTIN_KEYWORD_HELPERS = {
action,
array,
concat,
fn,
get,
hash,
if: inlineIf,
log,
mut,
'query-params': queryParams,
readonly,
unbound,
unless: inlineUnless,
'query-params': queryParams,
'-hash': hash,
'-each-in': eachIn,
'-normalize-class': normalizeClassHelper,
Expand All @@ -202,8 +197,21 @@ const BUILTINS_HELPERS = {
'-in-el-null': inElementNullCheckHelper,
};

const BUILTINS_MODIFIERS = {
const BUILTIN_HELPERS = {
...BUILTIN_KEYWORD_HELPERS,
array,
concat,
fn,
get,
hash,
};

const BUILTIN_KEYWORD_MODIFIERS = {
action: actionModifier,
};

const BUILTIN_MODIFIERS = {
...BUILTIN_KEYWORD_MODIFIERS,
on: onModifier,
};

Expand All @@ -226,10 +234,10 @@ export default class ResolverImpl implements RuntimeResolver<Owner>, CompileTime
lookupHelper(name: string, owner: Owner): Option<HelperDefinitionState> {
assert(
`You attempted to overwrite the built-in helper "${name}" which is not allowed. Please rename the helper.`,
!(BUILTINS_HELPERS[name] && owner.hasRegistration(`helper:${name}`))
!(BUILTIN_HELPERS[name] && owner.hasRegistration(`helper:${name}`))
);

const helper = BUILTINS_HELPERS[name];
const helper = BUILTIN_HELPERS[name];
if (helper !== undefined) {
return helper;
}
Expand Down Expand Up @@ -271,8 +279,12 @@ export default class ResolverImpl implements RuntimeResolver<Owner>, CompileTime
return definition;
}

lookupBuiltInHelper(name: string): HelperDefinitionState | null {
return BUILTIN_KEYWORD_HELPERS[name] ?? null;
}

lookupModifier(name: string, owner: Owner): Option<ModifierDefinitionState> {
let builtin = BUILTINS_MODIFIERS[name];
let builtin = BUILTIN_MODIFIERS[name];

if (builtin !== undefined) {
return builtin;
Expand All @@ -287,6 +299,10 @@ export default class ResolverImpl implements RuntimeResolver<Owner>, CompileTime
return modifier.class || null;
}

lookupBuiltInModifier(name: string): ModifierDefinitionState | null {
return BUILTIN_KEYWORD_MODIFIERS[name] ?? null;
}

lookupComponent(name: string, owner: Owner): ResolvedComponentDefinition | null {
let pair = lookupComponentPair(owner, name);

Expand Down Expand Up @@ -328,9 +344,12 @@ export default class ResolverImpl implements RuntimeResolver<Owner>, CompileTime
template,
};
} else {
let factory = owner.factoryFor(P`component:-default`)!;
let manager = getInternalComponentManager(owner, factory.class as object);

definition = {
state: owner.factoryFor(P`component:-default`)!,
manager: CURLY_COMPONENT_MANAGER,
state: factory,
manager,
template,
};
}
Expand All @@ -342,7 +361,7 @@ export default class ResolverImpl implements RuntimeResolver<Owner>, CompileTime
let manager = getInternalComponentManager(owner, ComponentClass);

definition = {
state: manager === CURLY_COMPONENT_MANAGER ? factory : ComponentClass,
state: isCurlyManager(manager) ? factory : ComponentClass,
manager,
template,
};
Expand Down
10 changes: 4 additions & 6 deletions packages/@ember/-internals/glimmer/lib/setup-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,18 @@ import { privatize as P, Registry } from '@ember/-internals/container';
import { ENV } from '@ember/-internals/environment';
import Component from './component';
import Checkbox from './components/checkbox';
import Input from './components/input';
import { InputComponent } from './components/input';
import LinkToComponent from './components/link-to';
import TextField from './components/text-field';
import TextArea from './components/textarea';
import { clientBuilder, rehydrationBuilder, serializeBuilder } from './dom';
import loc from './helpers/loc';
import { InertRenderer, InteractiveRenderer } from './renderer';
import InputTemplate from './templates/input';
import OutletTemplate from './templates/outlet';
import RootTemplate from './templates/root';
import OutletView from './views/outlet';

export function setupApplicationRegistry(registry: Registry) {
export function setupApplicationRegistry(registry: Registry): void {
registry.injection('renderer', 'env', '-environment:main');

// because we are using injections we can't use instantiate false
Expand Down Expand Up @@ -46,7 +45,7 @@ export function setupApplicationRegistry(registry: Registry) {
registry.injection('renderer', 'document', 'service:-document');
}

export function setupEngineRegistry(registry: Registry) {
export function setupEngineRegistry(registry: Registry): void {
registry.optionsForType('template', { instantiate: false });

registry.register('view:-outlet', OutletView);
Expand All @@ -61,8 +60,7 @@ export function setupEngineRegistry(registry: Registry) {
registry.register('component:-checkbox', Checkbox);
registry.register('component:link-to', LinkToComponent);

registry.register('component:input', Input);
registry.register('template:components/input', InputTemplate as any);
registry.register('component:input', InputComponent);

registry.register('component:textarea', TextArea);

Expand Down
Loading