Skip to content

Commit

Permalink
bugfix: Ensure Component Lookup Is Well Formed
Browse files Browse the repository at this point in the history
This PR introduces an assertion during the component name refinement to
assert that the lookup syntax is well formed. If a developer
accidentally types ":" instead of "::" you will end up with a runtime
erorr that looks like:

```
Uncaught Error: Assertion Failed: fullName must be a proper full name
    at assert (vendor.js:52729)
    at Container.lookup (vendor.js:16581)
    at Class.lookup (vendor.js:43950)
    at layoutFor (vendor.js:28680)
    at lookupComponentPair (vendor.js:28697)
    at lookupComponent (vendor.js:28711)
    at RuntimeResolver._lookupComponentDefinition (vendor.js:28992)
    at RuntimeResolver.lookupComponentHandle (vendor.js:28837)
    at CompileTimeLookup.lookupComponentDefinition (vendor.js:25065)
    at LazyCompiler.resolveLayoutForTag (vendor.js:60366)
```

This message is not actionable for developers. This PR will causes a
compile time error to occur with the following information:

```
Malformed component lookup in "test.js". Got <Foo:Bar \/> but you must use \"::\" to indicate a lookup
```
  • Loading branch information
chadhietala committed Jan 12, 2021
1 parent 2bb6445 commit ba57d78
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 0 deletions.
10 changes: 10 additions & 0 deletions packages/ember-template-compiler/lib/system/compile-options.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { EMBER_STRICT_MODE } from '@ember/canary-features';
import { assert } from '@ember/debug';
import { assign } from '@ember/polyfills';
import { PrecompileOptions } from '@glimmer/compiler';
import { AST, ASTPlugin, ASTPluginEnvironment, Syntax } from '@glimmer/syntax';
Expand All @@ -8,6 +9,10 @@ import COMPONENT_NAME_SIMPLE_DASHERIZE_CACHE from './dasherize-component-name';

let USER_PLUGINS: PluginFunc[] = [];

function malformedComponentLookup(string: string) {
return string.indexOf('::') === -1 && string.indexOf(':') > -1;
}

export default function compileOptions(
_options: Partial<EmberPrecompileOptions> = {}
): PrecompileOptions {
Expand All @@ -16,6 +21,11 @@ export default function compileOptions(
_options,
{
customizeComponentName(tagname: string): string {
assert(
`Malformed component lookup in "${_options.moduleName}". Got <${tagname} /> but you must use "::" to indicate a lookup`,
!malformedComponentLookup(tagname)
);

return COMPONENT_NAME_SIMPLE_DASHERIZE_CACHE.get(tagname);
},
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,16 @@ moduleFor(
assert.notEqual(compileOptions(), compileOptions());
}

['@test customizeComponentName asserts name is well formed'](assert) {
let options = compileOptions({ moduleName: 'test.js' });

expectAssertion(() => {
options.customizeComponentName('Foo:Bar');
}, /Malformed component lookup in "test.js". Got <Foo:Bar \/> but you must use "::" to indicate a lookup/);

assert.ok(options.customizeComponentName('Foo::Bar'));
}

['@test has default AST plugins in resolution mode'](assert) {
assert.expect(RESOLUTION_MODE_TRANSFORMS.length);

Expand Down

0 comments on commit ba57d78

Please sign in to comment.