Merge pull request #887 from HugoDF/feat-basic-error-handling

Feat: better (eval) error messages/context

examples/index.html

@@ -48,9 +48,11 @@
             </thead>
             <tbody>
                 <tr>
-                    <td>Broken Component</td>
+                    <td>Broken Components</td>
                     <td>
                         <div x-data="some.bad.expression()">I'm a broken component</div>
+                        <button x-data x-on:click="something()">I break on click</button>
+                        <div x-data x-spread="not.good">Broken x-spread</div>
                     </td>
                 </tr>
 

src/component.js

@@ -28,7 +28,7 @@ export default class Component {
             Object.defineProperty(dataExtras, `$${name}`, { get: function () { return callback(canonicalComponentElementReference) } });
         })
 
-        this.unobservedData = componentForClone ? componentForClone.getUnobservedData() : saferEval(dataExpression, dataExtras)
+        this.unobservedData = componentForClone ? componentForClone.getUnobservedData() : saferEval(el, dataExpression, dataExtras)
 
         /* IE11-ONLY:START */
             // For IE11, add our magic properties to the original data for access.
@@ -351,14 +351,14 @@ export default class Component {
     }
 
     evaluateReturnExpression(el, expression, extraVars = () => {}) {
-        return saferEval(expression, this.$data, {
+        return saferEval(el, expression, this.$data, {
             ...extraVars(),
             $dispatch: this.getDispatchFunction(el),
         })
     }
 
     evaluateCommandExpression(el, expression, extraVars = () => {}) {
-        return saferEvalNoReturn(expression, this.$data, {
+        return saferEvalNoReturn(el, expression, this.$data, {
             ...extraVars(),
             $dispatch: this.getDispatchFunction(el),
         })
@@ -390,7 +390,8 @@ export default class Component {
                 if (! (closestParentComponent && closestParentComponent.isSameNode(this.$el))) continue
 
                 if (mutations[i].type === 'attributes' && mutations[i].attributeName === 'x-data') {
-                    const rawData = saferEval(mutations[i].target.getAttribute('x-data') || '{}', { $el: this.$el })
+                    const xAttr = mutations[i].target.getAttribute('x-data') || '{}';
+                    const rawData = saferEval(this.$el, xAttr, { $el: this.$el })
 
                     Object.keys(rawData).forEach(key => {
                         if (this.$data[key] !== rawData[key]) {

src/utils.js

@@ -65,44 +65,66 @@ export function debounce(func, wait) {
     }
 }
 
-export function saferEval(expression, dataContext, additionalHelperVariables = {}) {
-    if (typeof expression === 'function') {
-        return expression.call(dataContext)
+const handleError = (el, expression, error) => {
+    console.error(`Alpine: error in expression "${expression}" in component:`, el, `due to "${error}"`);
+    if (! isTesting()) {
+        throw error;
     }
-
-    return (new Function(['$data', ...Object.keys(additionalHelperVariables)], `var __alpine_result; with($data) { __alpine_result = ${expression} }; return __alpine_result`))(
-        dataContext, ...Object.values(additionalHelperVariables)
-    )
 }
 
-export function saferEvalNoReturn(expression, dataContext, additionalHelperVariables = {}) {
-    if (typeof expression === 'function') {
-        return Promise.resolve(expression.call(dataContext, additionalHelperVariables['$event']))
+function tryCatch(cb, { el, expression }) {
+    try {
+        const value = cb();
+        return value instanceof Promise
+            ? value.catch((e) => handleError(el, expression, e))
+            : value;
+    } catch (e) {
+        handleError(el, expression, e)
     }
+}
 
-    let AsyncFunction = Function
-
-    /* MODERN-ONLY:START */
-        AsyncFunction = Object.getPrototypeOf(async function(){}).constructor
-    /* MODERN-ONLY:END */
+export function saferEval(el, expression, dataContext, additionalHelperVariables = {}) {
+    return tryCatch(() => {
+        if (typeof expression === 'function') {
+            return expression.call(dataContext)
+        }
 
-    // For the cases when users pass only a function reference to the caller: `x-on:click="foo"`
-    // Where "foo" is a function. Also, we'll pass the function the event instance when we call it.
-    if (Object.keys(dataContext).includes(expression)) {
-        let methodReference = (new Function(['dataContext', ...Object.keys(additionalHelperVariables)], `with(dataContext) { return ${expression} }`))(
+        return (new Function(['$data', ...Object.keys(additionalHelperVariables)], `var __alpine_result; with($data) { __alpine_result = ${expression} }; return __alpine_result`))(
             dataContext, ...Object.values(additionalHelperVariables)
         )
+    }, { el, expression })
+}
 
-        if (typeof methodReference === 'function') {
-            return Promise.resolve(methodReference.call(dataContext, additionalHelperVariables['$event']))
-        } else {
-            return Promise.resolve()
+export function saferEvalNoReturn(el, expression, dataContext, additionalHelperVariables = {}) {
+    return tryCatch(() => {
+        if (typeof expression === 'function') {
+            return Promise.resolve(expression.call(dataContext, additionalHelperVariables['$event']))
         }
-    }
 
-    return Promise.resolve((new AsyncFunction(['dataContext', ...Object.keys(additionalHelperVariables)], `with(dataContext) { ${expression} }`))(
-        dataContext, ...Object.values(additionalHelperVariables)
-    ))
+        let AsyncFunction = Function
+
+        /* MODERN-ONLY:START */
+            AsyncFunction = Object.getPrototypeOf(async function(){}).constructor
+        /* MODERN-ONLY:END */
+
+        // For the cases when users pass only a function reference to the caller: `x-on:click="foo"`
+        // Where "foo" is a function. Also, we'll pass the function the event instance when we call it.
+        if (Object.keys(dataContext).includes(expression)) {
+            let methodReference = (new Function(['dataContext', ...Object.keys(additionalHelperVariables)], `with(dataContext) { return ${expression} }`))(
+                dataContext, ...Object.values(additionalHelperVariables)
+            )
+
+            if (typeof methodReference === 'function') {
+                return Promise.resolve(methodReference.call(dataContext, additionalHelperVariables['$event']))
+            } else {
+                return Promise.resolve()
+            }
+        }
+
+        return Promise.resolve((new AsyncFunction(['dataContext', ...Object.keys(additionalHelperVariables)], `with(dataContext) { ${expression} }`))(
+            dataContext, ...Object.values(additionalHelperVariables)
+        ))
+    }, { el, expression })
 }
 
 const xAttrRE = /^x-(on|bind|data|text|html|model|if|for|show|cloak|transition|ref|spread)\b/
@@ -120,7 +142,7 @@ export function getXAttrs(el, component, type) {
     let spreadDirective = directives.filter(directive => directive.type === 'spread')[0]
 
     if (spreadDirective) {
-        let spreadObject = saferEval(spreadDirective.expression, component.$data)
+        let spreadObject = saferEval(el, spreadDirective.expression, component.$data)
 
         // Add x-spread directives to the pile of existing directives.
         directives = directives.concat(Object.entries(spreadObject).map(([name, value]) => parseHtmlAttribute({ name, value })))

test/error.spec.js

@@ -0,0 +1,119 @@
+import Alpine from 'alpinejs'
+import { wait } from '@testing-library/dom'
+
+global.MutationObserver = class {
+    observe() {}
+}
+
+jest.spyOn(window, 'setTimeout').mockImplementation((callback) => {
+    callback()
+})
+
+const mockConsoleError = jest.spyOn(console, 'error').mockImplementation(() => {})
+
+beforeEach(() => {
+    jest.clearAllMocks()
+})
+
+test('error in x-data eval contains element, expression and original error', async () => {
+    document.body.innerHTML = `
+        <div x-data="{ foo: 'bar' ">
+            <span x-bind:foo="foo.bar"></span>
+        </div>
+    `
+    await expect(Alpine.start()).rejects.toThrow()
+    expect(mockConsoleError).toHaveBeenCalledWith(
+        "Alpine: error in expression \"{ foo: 'bar' \" in component:",
+        document.querySelector('[x-data]'),
+        "due to \"SyntaxError: Unexpected token ')'\""
+    )
+})
+
+test('error in x-init eval contains element, expression and original error', async () => {
+    document.body.innerHTML = `
+        <div x-data x-init="foo.bar = 'baz'">
+        </div>
+    `
+    await Alpine.start()
+    expect(mockConsoleError).toHaveBeenCalledWith(
+        "Alpine: error in expression \"foo.bar = 'baz'\" in component:",
+        document.querySelector('[x-data]'),
+        "due to \"ReferenceError: foo is not defined\""
+    )
+})
+
+test('error in x-spread eval contains element, expression and original error', async () => {
+    document.body.innerHTML = `
+        <div x-data x-spread="foo.bar">
+        </div>
+    `
+    // swallow the rendering error
+    await expect(Alpine.start()).rejects.toThrow()
+    expect(mockConsoleError).toHaveBeenCalledWith(
+        "Alpine: error in expression \"foo.bar\" in component:",
+        document.querySelector('[x-data]'),
+        "due to \"ReferenceError: foo is not defined\""
+    )
+})
+
+test('error in x-bind eval contains element, expression and original error', async () => {
+    document.body.innerHTML = `
+        <div x-data="{ foo: null }">
+            <span x-bind:foo="foo.bar"></span>
+        </div>
+    `
+    await Alpine.start()
+    expect(mockConsoleError).toHaveBeenCalledWith(
+        "Alpine: error in expression \"foo.bar\" in component:",
+        document.querySelector('[x-bind:foo]'),
+        "due to \"TypeError: Cannot read property 'bar' of null\""
+    )
+})
+
+test('error in x-model eval contains element, expression and original error', async () => {
+    document.body.innerHTML = `
+        <div x-data="{ foo: null }">
+            <input x-model="foo.bar">
+        </div>
+    `
+    await Alpine.start()
+    expect(mockConsoleError).toHaveBeenCalledWith(
+        "Alpine: error in expression \"foo.bar\" in component:",
+        document.querySelector('[x-model]'),
+        "due to \"TypeError: Cannot read property 'bar' of null\""
+    )
+})
+
+test('error in x-for eval contains element, expression and original error', async () => {
+    document.body.innerHTML = `
+        <div x-data="{}">
+            <template x-for="element in foo">
+                <span x-text="element"></span>
+            </template>
+        </div>
+    `
+    await expect(Alpine.start()).rejects.toThrow()
+    expect(mockConsoleError).toHaveBeenCalledWith(
+        "Alpine: error in expression \"foo\" in component:",
+        document.querySelector('[x-for]'),
+        "due to \"ReferenceError: foo is not defined\""
+    )
+})
+
+test('error in x-on eval contains element, expression and original error', async () => {
+    document.body.innerHTML = `
+        <div
+            x-data="{hello: null}"
+            x-on:click="hello.world"
+        ></div>
+    `
+    await Alpine.start()
+    document.querySelector('div').click()
+    await wait(() => {
+        expect(mockConsoleError).toHaveBeenCalledWith(
+            "Alpine: error in expression \"hello.world\" in component:",
+            document.querySelector('[x-data]'),
+            "due to \"TypeError: Cannot read property 'world' of null\""
+        )
+    })
+})