- 
                Notifications
    
You must be signed in to change notification settings  - Fork 424
 
fix(engine): fixes issue #710 - add better error for invalid template #718
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -1,14 +1,14 @@ | ||
| import { Template } from "./template"; | ||
| 
     | 
||
| const VERIFIED_TEMPLATE_SET = new Set(); | ||
| const signedTemplateSet: Set<Template> = new Set(); | ||
| 
     | 
||
| export function isTemplateRegistered(tmpl: Template) { | ||
| if (!VERIFIED_TEMPLATE_SET.has(tmpl)) { | ||
| throw new TypeError('Unknown template'); | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. moving the throwing logic to the invoker of this method to we have access to the vm, and other details of the component to provide better message.  | 
||
| } | ||
| export function isTemplateRegistered(tpl: Template): boolean { | ||
| return signedTemplateSet.has(tpl); | ||
| } | ||
| 
     | 
||
| export function registerTemplate(tmpl: Template): Template { | ||
| VERIFIED_TEMPLATE_SET.add(tmpl); | ||
| return tmpl; | ||
| // chaining this method as a way to wrap existing | ||
| // assignment of templates easily, without too much transformation | ||
| export function registerTemplate(tpl: Template): Template { | ||
| signedTemplateSet.add(tpl); | ||
| return tpl; | ||
| } | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| 
          
            
          
           | 
    @@ -25,6 +25,12 @@ export interface Template { | |
| * This attribute is used for style encapsulation when the engine runs in fallback mode. | ||
| */ | ||
| shadowToken?: string; | ||
| 
     | 
||
| /** | ||
| * List of property names that are accessed of the component instance | ||
| * from the template. | ||
| */ | ||
| ids?: string[]; | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this interface was incompleted, surfaced the issue after adding the proper type to the html argument in   | 
||
| } | ||
| const EmptySlots: SlotSet = create(null); | ||
| 
     | 
||
| 
        
          
        
         | 
    @@ -44,7 +50,7 @@ function validateSlots(vm: VM, html: any) { | |
| } | ||
| } | ||
| 
     | 
||
| function validateFields(vm: VM, html: any) { | ||
| function validateFields(vm: VM, html: Template) { | ||
| if (process.env.NODE_ENV === 'production') { | ||
| // this method should never leak to prod | ||
| throw new ReferenceError(); | ||
| 
          
            
          
           | 
    @@ -99,8 +105,10 @@ export function evaluateTemplate(vm: VM, html: Template): Array<VNode|null> { | |
| resetShadowRoot(vm); | ||
| } | ||
| 
     | 
||
| // Check that the template is built by the compiler | ||
| isTemplateRegistered(html); | ||
| // Check that the template was built by the compiler | ||
| if (!isTemplateRegistered(html)) { | ||
| throw new ReferenceError(`The template rendered by ${vm} must return an imported template tag (e.g.: \`import html from "./${vm.def.name}.html"\`) or undefined, instead, it has returned a function.`); | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't need to add the component stack here since the invocation of this function is inside a try/catch in invoker, that's the one that will add the error details.  | 
||
| } | ||
| 
     | 
||
| vm.cmpTemplate = html; | ||
| 
     | 
||
| 
          
            
          
           | 
    ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just renaming and typing