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

Implement RFC#995, Deprecate component template resolution #20660

Merged
merged 32 commits into from Apr 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
880640f
Deprecate component template resolution
NullVoxPopuli Mar 13, 2024
2ab758f
Re-work how components are built in the internal test helpers
NullVoxPopuli Apr 2, 2024
d75d40b
Down to 52 failing tests
NullVoxPopuli Apr 5, 2024
9bb9c1c
40 left
NullVoxPopuli Apr 5, 2024
85255bc
Add checkbox to enable deprecations
NullVoxPopuli Apr 5, 2024
4ac5332
728 failing tests with the deprecations turned on
NullVoxPopuli Apr 5, 2024
8314402
Down to 13
NullVoxPopuli Apr 5, 2024
c7508cc
Base tests pass again after test harness updates
NullVoxPopuli Apr 5, 2024
f0b3ec3
Update the test-resolver-application
NullVoxPopuli Apr 5, 2024
53f346a
Add attempted lookup in error message
NullVoxPopuli Apr 5, 2024
2a9722e
Wip try to fix debug render tree tests
NullVoxPopuli Apr 5, 2024
9453279
Update lookupComponent and lookupComponentPair to not try to resolve …
NullVoxPopuli Apr 8, 2024
ae90597
progress
NullVoxPopuli Apr 8, 2024
1cdf2d7
Progress
NullVoxPopuli Apr 8, 2024
80a62a6
Progress
NullVoxPopuli Apr 8, 2024
3851dc2
4 tests left
NullVoxPopuli Apr 8, 2024
664bab0
lints
NullVoxPopuli Apr 8, 2024
ea8d156
Done?
NullVoxPopuli Apr 8, 2024
563ddc0
'Fix' type errors
NullVoxPopuli Apr 8, 2024
14eb06c
I think everything is green?
NullVoxPopuli Apr 9, 2024
4f90a1a
Make smoke-tests pass again
NullVoxPopuli Apr 9, 2024
178e891
idk how to fix this
NullVoxPopuli Apr 9, 2024
10f2d8c
Yay
NullVoxPopuli Apr 10, 2024
4985f91
Remove code from curly manager that is no longer needed
NullVoxPopuli Apr 18, 2024
869b733
Change from isRemoved() to .isRemoved
NullVoxPopuli Apr 18, 2024
5c11fd2
cleanup
NullVoxPopuli Apr 18, 2024
2fe2ec9
Move deprecation to be if layoutFor has a result only
NullVoxPopuli Apr 19, 2024
06268ce
Add a non-deprecated version of the append lifecycle test
NullVoxPopuli Apr 19, 2024
79ede0e
Re-fix upstream-changed test
NullVoxPopuli Apr 23, 2024
f17365f
Move early nullification to layoutFor
NullVoxPopuli Apr 25, 2024
52978cd
Remove unneeded branch
NullVoxPopuli Apr 25, 2024
eb8fc81
Fix qunit param
NullVoxPopuli Apr 25, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions packages/@ember/-internals/deprecations/index.ts
Expand Up @@ -106,6 +106,15 @@ export const DEPRECATIONS = {
available: '5.9.0',
},
}),
DEPRECATE_COMPONENT_TEMPLATE_RESOLVING: deprecation({
id: 'component-template-resolving',
url: 'https://deprecations.emberjs.com/id/component-template-resolving',
until: '6.0.0',
for: 'ember-source',
since: {
available: '5.9.0',
},
}),
};

export function deprecateUntil(message: string, deprecation: DeprecationObject) {
Expand Down
16 changes: 15 additions & 1 deletion packages/@ember/-internals/glimmer/lib/resolver.ts
Expand Up @@ -45,6 +45,7 @@ import { default as uniqueId } from './helpers/unique-id';
import actionModifier from './modifiers/action';
import { mountHelper } from './syntax/mount';
import { outletHelper } from './syntax/outlet';
import { DEPRECATIONS, deprecateUntil } from '@ember/-internals/deprecations';

function instrumentationPayload(name: string) {
return { object: `component:${name}` };
Expand All @@ -63,9 +64,22 @@ function layoutFor(
owner: InternalOwner,
options?: RegisterOptions
): Nullable<Template> {
if (DEPRECATIONS.DEPRECATE_COMPONENT_TEMPLATE_RESOLVING.isRemoved) {
return null;
}

let templateFullName = `template:components/${name}` as const;

return (owner.lookup(templateFullName, options) as Template) || null;
let result = (owner.lookup(templateFullName, options) as Template) || null;

if (result) {
deprecateUntil(
`Components with separately resolved templates are deprecated. Migrate to either co-located js/ts + hbs files or to gjs/gts. Tried to lookup '${templateFullName}'.`,
DEPRECATIONS.DEPRECATE_COMPONENT_TEMPLATE_RESOLVING
);
}

return result;
}

type LookupResult =
Expand Down
@@ -1,8 +1,10 @@
import {
ApplicationTestCase,
ModuleBasedTestResolver,
expectDeprecation,
moduleFor,
strip,
testUnless,
} from 'internal-test-helpers';

import { ENV } from '@ember/-internals/environment';
Expand All @@ -22,6 +24,8 @@ import type { SimpleElement, SimpleNode } from '@simple-dom/interface';
import type { EmberPrecompileOptions } from 'ember-template-compiler';
import { compile } from 'ember-template-compiler';
import { runTask } from 'internal-test-helpers/lib/run';
import { DEPRECATIONS } from '@ember/-internals/deprecations';
import templateOnly from '@ember/component/template-only';

interface CapturedBounds {
parentElement: SimpleElement;
Expand Down Expand Up @@ -204,10 +208,13 @@ if (ENV._DEBUG_RENDER_TREE) {
)
);
this.register(
'template:components/inspect-model',
compileTemplate('{{@model}}', {
moduleName: 'foo/components/inspect-model.hbs',
})
'component:inspect-model',
setComponentTemplate(
compileTemplate('{{@model}}', {
moduleName: 'foo/components/inspect-model.hbs',
}),
templateOnly()
)
);
}

Expand Down Expand Up @@ -242,10 +249,13 @@ if (ENV._DEBUG_RENDER_TREE) {
)
);
this.register(
'template:components/inspect-model',
compileTemplate('{{@model}}', {
moduleName: 'bar/components/inspect-model.hbs',
})
'component:inspect-model',
setComponentTemplate(
compileTemplate('{{@model}}', {
moduleName: 'bar/components/inspect-model.hbs',
}),
templateOnly()
)
);
}

Expand Down Expand Up @@ -608,10 +618,13 @@ if (ENV._DEBUG_RENDER_TREE) {
})
);
this.register(
'template:components/hello',
compileTemplate('<span>Hello {{@message}}</span>', {
moduleName: 'foo/components/hello.hbs',
})
'component:hello',
setComponentTemplate(
compileTemplate('<span>Hello {{@message}}</span>', {
moduleName: 'foo/components/hello.hbs',
}),
templateOnlyComponent()
)
);
}

Expand Down Expand Up @@ -775,7 +788,13 @@ if (ENV._DEBUG_RENDER_TREE) {
]);
}

async '@test template-only components'() {
async [`${testUnless(
DEPRECATIONS.DEPRECATE_COMPONENT_TEMPLATE_RESOLVING.isRemoved
)} template-only components`]() {
expectDeprecation(
/resolved templates/,
DEPRECATIONS.DEPRECATE_COMPONENT_TEMPLATE_RESOLVING.isEnabled
);
this.addTemplate(
'application',
strip`
Expand All @@ -800,7 +819,7 @@ if (ENV._DEBUG_RENDER_TREE) {
name: 'hello-world',
args: { positional: [], named: { name: 'first' } },
instance: null,
template: 'my-app/templates/components/hello-world.hbs',
Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

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

this field is based of moduleName, which we've talked about getting rid of.

It still works when a template is resolved, but when using setComponentTemplate, there is no module name to associate -- could come from anywhere

template: '(unknown template module)',
bounds: this.nodeBounds(this.element!.firstChild),
children: [],
},
Expand All @@ -816,7 +835,7 @@ if (ENV._DEBUG_RENDER_TREE) {
name: 'hello-world',
args: { positional: [], named: { name: 'first' } },
instance: null,
template: 'my-app/templates/components/hello-world.hbs',
template: '(unknown template module)',
bounds: this.nodeBounds(this.element!.firstChild),
children: [],
},
Expand All @@ -825,7 +844,7 @@ if (ENV._DEBUG_RENDER_TREE) {
name: 'hello-world',
args: { positional: [], named: { name: 'second' } },
instance: null,
template: 'my-app/templates/components/hello-world.hbs',
template: '(unknown template module)',
bounds: this.nodeBounds(this.element!.lastChild),
children: [],
},
Expand All @@ -841,7 +860,7 @@ if (ENV._DEBUG_RENDER_TREE) {
name: 'hello-world',
args: { positional: [], named: { name: 'first' } },
instance: null,
template: 'my-app/templates/components/hello-world.hbs',
template: '(unknown template module)',
bounds: this.nodeBounds(this.element!.firstChild),
children: [],
},
Expand Down Expand Up @@ -873,7 +892,7 @@ if (ENV._DEBUG_RENDER_TREE) {
name: 'hello-world',
args: { positional: [], named: { name: 'first' } },
instance: null,
template: 'my-app/templates/components/hello-world.hbs',
template: '(unknown template module)',
bounds: this.nodeBounds(this.element!.firstChild),
children: [],
},
Expand All @@ -889,7 +908,7 @@ if (ENV._DEBUG_RENDER_TREE) {
name: 'hello-world',
args: { positional: [], named: { name: 'first' } },
instance: null,
template: 'my-app/templates/components/hello-world.hbs',
template: '(unknown template module)',
bounds: this.nodeBounds(this.element!.firstChild),
children: [],
},
Expand All @@ -898,7 +917,7 @@ if (ENV._DEBUG_RENDER_TREE) {
name: 'hello-world',
args: { positional: [], named: { name: 'second' } },
instance: null,
template: 'my-app/templates/components/hello-world.hbs',
template: '(unknown template module)',
bounds: this.nodeBounds(this.element!.lastChild),
children: [],
},
Expand All @@ -914,7 +933,7 @@ if (ENV._DEBUG_RENDER_TREE) {
name: 'hello-world',
args: { positional: [], named: { name: 'first' } },
instance: null,
template: 'my-app/templates/components/hello-world.hbs',
template: '(unknown template module)',
bounds: this.nodeBounds(this.element!.firstChild),
children: [],
},
Expand All @@ -936,7 +955,7 @@ if (ENV._DEBUG_RENDER_TREE) {
this.addComponent('hello-world', {
ComponentClass: setComponentTemplate(
compileTemplate('{{@name}}', { moduleName: 'my-app/components/hello-world.hbs' }),
templateOnlyComponent()
templateOnlyComponent('my-app/components/hello-world', 'HelloWorld')
),
});

Expand Down Expand Up @@ -1021,7 +1040,7 @@ if (ENV._DEBUG_RENDER_TREE) {
name: 'hello-world',
args: { positional: [], named: { name: 'first' } },
instance: (instance: Record<string, string>) => instance['name'] === 'first',
template: 'my-app/templates/components/hello-world.hbs',
template: '(unknown template module)',
bounds: this.nodeBounds(this.element!.firstChild),
children: [],
},
Expand All @@ -1037,7 +1056,7 @@ if (ENV._DEBUG_RENDER_TREE) {
name: 'hello-world',
args: { positional: [], named: { name: 'first' } },
instance: (instance: Record<string, string>) => instance['name'] === 'first',
template: 'my-app/templates/components/hello-world.hbs',
template: '(unknown template module)',
bounds: this.nodeBounds(this.element!.firstChild),
children: [],
},
Expand All @@ -1046,7 +1065,7 @@ if (ENV._DEBUG_RENDER_TREE) {
name: 'hello-world',
args: { positional: [], named: { name: 'second' } },
instance: (instance: Record<string, string>) => instance['name'] === 'second',
template: 'my-app/templates/components/hello-world.hbs',
template: '(unknown template module)',
bounds: this.nodeBounds(this.element!.lastChild),
children: [],
},
Expand All @@ -1062,7 +1081,7 @@ if (ENV._DEBUG_RENDER_TREE) {
name: 'hello-world',
args: { positional: [], named: { name: 'first' } },
instance: (instance: Record<string, string>) => instance['name'] === 'first',
template: 'my-app/templates/components/hello-world.hbs',
template: '(unknown template module)',
bounds: this.nodeBounds(this.element!.firstChild),
children: [],
},
Expand Down Expand Up @@ -1106,7 +1125,7 @@ if (ENV._DEBUG_RENDER_TREE) {
name: 'hello-world',
args: { positional: [], named: { name: 'first' } },
instance: (instance: Record<string, string>) => instance['name'] === 'first',
template: 'my-app/templates/components/hello-world.hbs',
template: '(unknown template module)',
bounds: this.nodeBounds(this.element!.firstChild),
children: [],
},
Expand All @@ -1122,7 +1141,7 @@ if (ENV._DEBUG_RENDER_TREE) {
name: 'hello-world',
args: { positional: [], named: { name: 'first' } },
instance: (instance: Record<string, string>) => instance['name'] === 'first',
template: 'my-app/templates/components/hello-world.hbs',
template: '(unknown template module)',
bounds: this.nodeBounds(this.element!.firstChild),
children: [],
},
Expand All @@ -1131,7 +1150,7 @@ if (ENV._DEBUG_RENDER_TREE) {
name: 'hello-world',
args: { positional: [], named: { name: 'second' } },
instance: (instance: Record<string, string>) => instance['name'] === 'second',
template: 'my-app/templates/components/hello-world.hbs',
template: '(unknown template module)',
bounds: this.nodeBounds(this.element!.lastChild),
children: [],
},
Expand All @@ -1147,7 +1166,7 @@ if (ENV._DEBUG_RENDER_TREE) {
name: 'hello-world',
args: { positional: [], named: { name: 'first' } },
instance: (instance: Record<string, string>) => instance['name'] === 'first',
template: 'my-app/templates/components/hello-world.hbs',
template: '(unknown template module)',
bounds: this.nodeBounds(this.element!.firstChild),
children: [],
},
Expand Down
Expand Up @@ -14,6 +14,8 @@ import Engine from '@ember/engine';
import { next } from '@ember/runloop';

import { compile } from '../../utils/helpers';
import { setComponentTemplate } from '@glimmer/manager';
import { templateOnlyComponent } from '@glimmer/runtime';

moduleFor(
'Application test: engine rendering',
Expand Down Expand Up @@ -217,10 +219,8 @@ moduleFor(
);
this.register('template:application', sharedTemplate);
this.register(
'template:components/ambiguous-curlies',
compile(strip`
<p>Component!</p>
`)
'component:ambiguous-curlies',
setComponentTemplate(compile(`<p>Component!</p>`), templateOnlyComponent())
);
},
})
Expand Down Expand Up @@ -275,10 +275,8 @@ moduleFor(
);
this.register('component:my-component', sharedComponent);
this.register(
'template:components/ambiguous-curlies',
compile(strip`
<p>Component!</p>
`)
'component:ambiguous-curlies',
setComponentTemplate(compile(`<p>Component!</p>`), templateOnlyComponent())
);
},
})
Expand Down