Skip to content

Commit

Permalink
[BUGFIX beta] internal templates should be strictMode
Browse files Browse the repository at this point in the history
As of #20587, the modules published under `dist/packages` act more like normal addon code,  in that their templates are published as calls to `precompileTemplate` rather than wire format.

But that makes this the first time that Embroider is seeing these templates, and because they are in loose mode and use the dynamic component helper they are un-analyzable.

This PR:
 - switches them to strict mode
 - fixes the declared types for `precompileTemplate`, which were a lie before. `scope` is allowed in non-strict mode. `scope` is optional even in strict mode.
 - fixes a spelling error in a recently-introduced internal method, because I noticed it and it annoyed me.
  • Loading branch information
ef4 committed Dec 12, 2023
1 parent d877ecc commit 82b5b1d
Show file tree
Hide file tree
Showing 12 changed files with 51 additions and 29 deletions.
1 change: 1 addition & 0 deletions packages/@ember/-internals/glimmer/lib/templates/empty.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { precompileTemplate } from '@ember/template-compilation';
export default precompileTemplate('', {
moduleName: 'packages/@ember/-internals/glimmer/lib/templates/empty.hbs',
strictMode: true,
});
9 changes: 8 additions & 1 deletion packages/@ember/-internals/glimmer/lib/templates/input.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { precompileTemplate } from '@ember/template-compilation';
import { on } from '@ember/modifier';
export default precompileTemplate(
`<input
{{!-- for compatibility --}}
Expand All @@ -17,5 +18,11 @@ export default precompileTemplate(
{{on "paste" this.valueDidChange}}
{{on "cut" this.valueDidChange}}
/>`,
{ moduleName: 'packages/@ember/-internals/glimmer/lib/templates/input.hbs' }
{
moduleName: 'packages/@ember/-internals/glimmer/lib/templates/input.hbs',
strictMode: true,
scope() {
return { on };
},
}
);
10 changes: 9 additions & 1 deletion packages/@ember/-internals/glimmer/lib/templates/link-to.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { precompileTemplate } from '@ember/template-compilation';
import { on } from '@ember/modifier';

export default precompileTemplate(
`<a
{{!-- for compatibility --}}
Expand All @@ -18,5 +20,11 @@ export default precompileTemplate(
{{on 'click' this.click}}
>{{yield}}</a>`,
{ moduleName: 'packages/@ember/-internals/glimmer/lib/templates/link-to.hbs' }
{
moduleName: 'packages/@ember/-internals/glimmer/lib/templates/link-to.hbs',
strictMode: true,
scope() {
return { on };
},
}
);
8 changes: 7 additions & 1 deletion packages/@ember/-internals/glimmer/lib/templates/outlet.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import { precompileTemplate } from '@ember/template-compilation';
export default precompileTemplate(`{{component (-outlet)}}`, {
import { outletHelper } from '../syntax/outlet';

export default precompileTemplate(`{{component (outletHelper)}}`, {
moduleName: 'packages/@ember/-internals/glimmer/lib/templates/outlet.hbs',
strictMode: true,
scope() {
return { outletHelper };
},
});
1 change: 1 addition & 0 deletions packages/@ember/-internals/glimmer/lib/templates/root.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { precompileTemplate } from '@ember/template-compilation';
export default precompileTemplate(`{{component this}}`, {
moduleName: 'packages/@ember/-internals/glimmer/lib/templates/root.hbs',
strictMode: true,
});
10 changes: 9 additions & 1 deletion packages/@ember/-internals/glimmer/lib/templates/textarea.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { precompileTemplate } from '@ember/template-compilation';
import { on } from '@ember/modifier';

export default precompileTemplate(
`<textarea
{{!-- for compatibility --}}
Expand All @@ -15,5 +17,11 @@ export default precompileTemplate(
{{on "paste" this.valueDidChange}}
{{on "cut" this.valueDidChange}}
/>`,
{ moduleName: 'packages/@ember/-internals/glimmer/lib/templates/textarea.hbs' }
{
moduleName: 'packages/@ember/-internals/glimmer/lib/templates/textarea.hbs',
strictMode: true,
scope() {
return { on };
},
}
);
1 change: 1 addition & 0 deletions packages/@ember/-internals/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"@ember/enumerable": "workspace:*",
"@ember/helper": "workspace:*",
"@ember/instrumentation": "workspace:*",
"@ember/modifier": "workspace:*",
"@ember/object": "workspace:*",
"@ember/owner": "workspace:*",
"@ember/routing": "workspace:*",
Expand Down
23 changes: 8 additions & 15 deletions packages/@ember/template-compilation/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,21 @@ import { DEBUG } from '@glimmer/env';
import type { TemplateFactory } from '@glimmer/interfaces';
import type * as ETC from 'ember-template-compiler';

interface CommonOptions {
moduleName?: string;
}

interface LooseModeOptions extends CommonOptions {
strictMode?: false;
}

interface StrictModeOptions extends CommonOptions {
strictMode: true;
scope: () => Record<string, unknown>;
}

// (UN)SAFETY: the public API is that people can import and use this (and indeed
// it is emitted as part of Ember's build!), so we define it as having the type
// which makes that work. However, in practice it is supplied by the build,
// *for* the build, and will *not* be present at runtime, so the actual value
// here is `undefined` in prod; in dev it is a function which throws a somewhat
// nicer error. This is janky, but... here we are.
interface PrecompileTemplate {
(templateString: string, options?: LooseModeOptions): TemplateFactory;
(templateString: string, options: StrictModeOptions): TemplateFactory;
(
templateString: string,
options?: {
strictMode?: boolean;
scope?: () => Record<string, unknown>;
moduleName?: string;
}
): TemplateFactory;
}

export let __emberTemplateCompiler: undefined | typeof ETC;
Expand Down
2 changes: 1 addition & 1 deletion packages/@ember/test/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ registerWaiter = testingNotAvailableMessage;
unregisterHelper = testingNotAvailableMessage;
unregisterWaiter = testingNotAvailableMessage;

export function registerTestImplementaiton(impl: typeof EmberTesting) {
export function registerTestImplementation(impl: typeof EmberTesting) {
let { Test } = impl;
registerAsyncHelper = Test.registerAsyncHelper;
registerHelper = Test.registerHelper;
Expand Down
4 changes: 2 additions & 2 deletions packages/ember-testing/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
export * from './lib/public-api';
import * as EmberTesting from './lib/public-api';
import { registerTestImplementaiton } from '@ember/test';
import { registerTestImplementation } from '@ember/test';

registerTestImplementaiton(EmberTesting);
registerTestImplementation(EmberTesting);
5 changes: 4 additions & 1 deletion pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 0 additions & 6 deletions type-tests/@ember/template-compilation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@ precompileTemplate(`Hello World`, { strictMode: true, moduleName: 'hello', scope
// Integration, since this is the primary use case for precompileTemplate
expectTypeOf(setComponentTemplate(precompileTemplate(`Hello World`), templateOnly())).toBeObject();

// @ts-expect-error scope is required when strictMode is true
precompileTemplate(`Hello World`, { strictMode: true });

// @ts-expect-error scope must be a function
precompileTemplate(`Hello World`, { strictMode: true, scope: {} });

Expand All @@ -26,6 +23,3 @@ precompileTemplate(`Hello World`, { strictMode: true, scope: () => {} });

// @ts-expect-error scope must return an object and arrays are not the kind of object we want
precompileTemplate(`Hello World`, { strictMode: true, scope: () => [] });

// @ts-expect-error scope has no purpose when strictMode is false
precompileTemplate(`Hello World`, { strictMode: false, scope: () => ({}) });

0 comments on commit 82b5b1d

Please sign in to comment.