Skip to content

Commit c5ffa11

Browse files
Jacob Pozaicdanddanddand
Jacob Pozaic
and
danddanddand
authored
Improved Alpine error resiliency and logging (alpinejs#2027)
* Iteration on error logging, adding event on errors for plugins to listen to. * Iteration on error logging, adding event on errors for plugins to listen to. ( pulled package-lock.json ) * Iteration on error logging, adding event on errors for plugins to listen to. -> fixed import from csp * move el and expression onto detail object directly * add back error for better dev experience * fix two bad tests that fail when exception handling is working * remove test since empty x-init is not valid * remove error event dispatching * remove unnecessary try/catch that should be handled internally * Fixed extra tab * Removed unused import * prevent useless 'func is not a function' errors Co-authored-by: jacobp <mozillafirefox1> Co-authored-by: dand <[email protected]>
1 parent 52ff275 commit c5ffa11

File tree

8 files changed

+272
-41
lines changed

8 files changed

+272
-41
lines changed

‎packages/alpinejs/src/directives/x-data.js

+4
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@ directive('data', skipDuringClone((el, { expression }, { cleanup }) => {
2121

2222
let data = evaluate(el, expression, { scope: dataProviderContext })
2323

24+
if( data === undefined ) {
25+
data = {}
26+
}
27+
2428
injectMagics(data, el)
2529

2630
let reactiveData = reactive(data)

‎packages/alpinejs/src/evaluator.js

+29-28
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { closestDataStack, mergeProxies } from './scope'
22
import { injectMagics } from './magics'
3+
import { tryCatch, handleError } from './utils/error'
34

45
export function evaluate(el, expression, extras = {}) {
56
let result
@@ -30,7 +31,7 @@ export function normalEvaluator(el, expression) {
3031
return generateEvaluatorFromFunction(dataStack, expression)
3132
}
3233

33-
let evaluator = generateEvaluatorFromString(dataStack, expression)
34+
let evaluator = generateEvaluatorFromString(dataStack, expression, el)
3435

3536
return tryCatch.bind(null, el, expression, evaluator)
3637
}
@@ -45,7 +46,7 @@ export function generateEvaluatorFromFunction(dataStack, func) {
4546

4647
let evaluatorMemo = {}
4748

48-
function generateFunctionFromString(expression) {
49+
function generateFunctionFromString(expression, el) {
4950
if (evaluatorMemo[expression]) {
5051
return evaluatorMemo[expression]
5152
}
@@ -63,15 +64,23 @@ function generateFunctionFromString(expression) {
6364
? `(() => { ${expression} })()`
6465
: expression
6566

66-
let func = new AsyncFunction(['__self', 'scope'], `with (scope) { __self.result = ${rightSideSafeExpression} }; __self.finished = true; return __self.result;`)
67+
const safeAsyncFunction = () => {
68+
try {
69+
return new AsyncFunction(['__self', 'scope'], `with (scope) { __self.result = ${rightSideSafeExpression} }; __self.finished = true; return __self.result;`)
70+
} catch ( error ) {
71+
handleError( error, el, expression )
72+
return Promise.resolve()
73+
}
74+
}
75+
let func = safeAsyncFunction()
6776

6877
evaluatorMemo[expression] = func
6978

7079
return func
7180
}
7281

73-
function generateEvaluatorFromString(dataStack, expression) {
74-
let func = generateFunctionFromString(expression)
82+
function generateEvaluatorFromString(dataStack, expression, el) {
83+
let func = generateFunctionFromString(expression, el)
7584

7685
return (receiver = () => {}, { scope = {}, params = [] } = {}) => {
7786
func.result = undefined
@@ -81,41 +90,33 @@ function generateEvaluatorFromString(dataStack, expression) {
8190

8291
let completeScope = mergeProxies([ scope, ...dataStack ])
8392

84-
let promise = func(func, completeScope)
85-
86-
// Check if the function ran synchronously,
87-
if (func.finished) {
88-
// Return the immediate result.
89-
runIfTypeOfFunction(receiver, func.result, completeScope, params)
90-
} else {
91-
// If not, return the result when the promise resolves.
92-
promise.then(result => {
93-
runIfTypeOfFunction(receiver, result, completeScope, params)
94-
})
93+
if( typeof func === 'function' ) {
94+
let promise = func(func, completeScope).catch((error) => handleError(error, el, expression))
95+
96+
// Check if the function ran synchronously,
97+
if (func.finished) {
98+
// Return the immediate result.
99+
runIfTypeOfFunction(receiver, func.result, completeScope, params, el)
100+
} else {
101+
// If not, return the result when the promise resolves.
102+
promise.then(result => {
103+
runIfTypeOfFunction(receiver, result, completeScope, params, el)
104+
}).catch( error => handleError( error, el, expression ) )
105+
}
95106
}
96107
}
97108
}
98109

99-
export function runIfTypeOfFunction(receiver, value, scope, params) {
110+
export function runIfTypeOfFunction(receiver, value, scope, params, el) {
100111
if (typeof value === 'function') {
101112
let result = value.apply(scope, params)
102113

103114
if (result instanceof Promise) {
104-
result.then(i => runIfTypeOfFunction(receiver, i, scope, params))
115+
result.then(i => runIfTypeOfFunction(receiver, i, scope, params)).catch( error => handleError( error, el, value ) )
105116
} else {
106117
receiver(result)
107118
}
108119
} else {
109120
receiver(value)
110121
}
111122
}
112-
113-
export function tryCatch(el, expression, callback, ...args) {
114-
try {
115-
return callback(...args)
116-
} catch (e) {
117-
console.warn(`Alpine Expression Error: ${e.message}\n\nExpression: "${expression}"\n\n`, el)
118-
119-
throw e
120-
}
121-
}

‎packages/alpinejs/src/utils/error.js

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
export function tryCatch(el, expression, callback, ...args) {
2+
try {
3+
return callback(...args)
4+
} catch (e) {
5+
handleError( e, el, expression )
6+
}
7+
}
8+
9+
export function handleError(error, el, expression = undefined) {
10+
Object.assign( error, { el, expression } )
11+
12+
console.warn(`Alpine Expression Error: ${error.message}\n\n${ expression ? 'Expression: \"' + expression + '\"\n\n' : '' }`, el)
13+
14+
setTimeout( () => { throw error }, 0 )
15+
}

‎packages/csp/src/index.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ import 'alpinejs/src/directives/index'
1010

1111
import { closestDataStack, mergeProxies } from 'alpinejs/src/scope'
1212
import { injectMagics } from 'alpinejs/src/magics'
13-
import { generateEvaluatorFromFunction, runIfTypeOfFunction, tryCatch } from 'alpinejs/src/evaluator'
13+
import { generateEvaluatorFromFunction, runIfTypeOfFunction } from 'alpinejs/src/evaluator'
14+
import { tryCatch } from 'alpinejs/src/utils/error'
1415

1516
function cspCompliantEvaluator(el, expression) {
1617
let overriddenMagics = {}

‎tests/cypress/integration/custom-directives.spec.js

-2
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ test('directives are auto cleaned up',
2626
`,
2727
`
2828
Alpine.directive('foo', (el, {}, { effect, cleanup, evaluateLater }) => {
29-
let evaluate = evaluateLater('foo')
3029
let incCount = evaluateLater('count++')
3130
3231
cleanup(() => {
@@ -36,7 +35,6 @@ test('directives are auto cleaned up',
3635
3736
effect(() => {
3837
incCount()
39-
evaluate(value => el.textContent = value)
4038
})
4139
})
4240
`],

‎tests/cypress/integration/error-handling.spec.js

+202
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,202 @@
1+
import { haveText, html, test } from '../utils'
2+
3+
export function setupConsoleInterceptor( ...targetIds ) {
4+
const mappedTargetIds = targetIds.map( tid => `'${tid}'` ).join( ',' )
5+
return `
6+
let errorContainer = document.createElement('div');
7+
errorContainer.id = 'errors'
8+
errorContainer.textContent = 'false'
9+
document.querySelector('#root').after(errorContainer)
10+
console.warnlog = console.warn.bind(console)
11+
console.warn = function () {
12+
document.getElementById( 'errors' ).textContent = [${mappedTargetIds}].some( target => arguments[1] === document.getElementById( target ) )
13+
console.warnlog.apply(console, arguments)
14+
}
15+
`
16+
}
17+
18+
export function assertConsoleInterceptorHadErrorWithCorrectElement() {
19+
return ({get}) => {
20+
get('#errors').should(haveText('true'))
21+
};
22+
}
23+
24+
test('x-for identifier issue',
25+
[html`
26+
<div x-data="{ items: ['foo'] }">
27+
<template id="xfor" x-for="item in itemzzzz">
28+
<span x-text="item"></span>
29+
</template>
30+
</div>
31+
`,
32+
setupConsoleInterceptor( "xfor" )
33+
],
34+
assertConsoleInterceptorHadErrorWithCorrectElement(),
35+
true
36+
)
37+
38+
test('x-text identifier issue',
39+
[html`
40+
<div x-data="{ items: ['foo'] }">
41+
<template x-for="item in items">
42+
<span id="xtext" x-text="itemzzz"></span>
43+
</template>
44+
</div>
45+
`,
46+
setupConsoleInterceptor( "xtext" )
47+
],
48+
assertConsoleInterceptorHadErrorWithCorrectElement(),
49+
true
50+
)
51+
52+
test('x-init identifier issue',
53+
[html`
54+
<div id="xinit" x-data x-init="doesNotExist()">
55+
</div>
56+
`,
57+
setupConsoleInterceptor( "xinit" )
58+
],
59+
assertConsoleInterceptorHadErrorWithCorrectElement(),
60+
true
61+
)
62+
63+
test('x-show identifier issue',
64+
[html`
65+
<div id="xshow" x-data="{isOpen: true}" x-show="isVisible">
66+
</div>
67+
`,
68+
setupConsoleInterceptor( "xshow" )
69+
],
70+
assertConsoleInterceptorHadErrorWithCorrectElement(),
71+
true
72+
)
73+
74+
test('x-bind class object syntax identifier issue',
75+
[html`
76+
<div x-data="{isOpen: true}">
77+
<div id="xbind" :class="{ 'block' : isVisible, 'hidden' : !isVisible }"></div>
78+
</div>
79+
`,
80+
setupConsoleInterceptor( "xbind" )
81+
],
82+
assertConsoleInterceptorHadErrorWithCorrectElement(),
83+
true
84+
)
85+
86+
test('x-model identifier issue',
87+
[html`
88+
<div x-data="{value: ''}">
89+
<input id="xmodel" x-model="thething"/>
90+
</div>
91+
`,
92+
setupConsoleInterceptor( "xmodel" )
93+
],
94+
assertConsoleInterceptorHadErrorWithCorrectElement(),
95+
true
96+
)
97+
98+
test('x-if identifier issue',
99+
[html`
100+
<div x-data="{value: ''}">
101+
<template id="xif" x-if="valuez === ''">
102+
<span>Words</span>
103+
</template>
104+
</div>
105+
`,
106+
setupConsoleInterceptor( "xif" )
107+
],
108+
assertConsoleInterceptorHadErrorWithCorrectElement(),
109+
true
110+
)
111+
112+
test('x-if identifier issue ( function )',
113+
[html`
114+
<div x-data="{shouldOpen: function(){}}">
115+
<template id="xif" x-if="isOpen()">
116+
<span>Words</span>
117+
</template>
118+
</div>
119+
`,
120+
setupConsoleInterceptor( "xif" )
121+
],
122+
assertConsoleInterceptorHadErrorWithCorrectElement(),
123+
true
124+
)
125+
126+
test('x-effect identifier issue',
127+
[html`
128+
<div id="xeffect" x-data="{ label: 'Hello' }" x-effect="System.out.println(label)">
129+
</div>
130+
`,
131+
setupConsoleInterceptor( "xeffect" )
132+
],
133+
assertConsoleInterceptorHadErrorWithCorrectElement(),
134+
true
135+
)
136+
137+
test('x-on identifier issue',
138+
[html`
139+
<div x-data="{ label: 'Hello' }">
140+
<div x-text="label"></div>
141+
<button id="xon" x-on:click="labelz += ' World!'">Change Message</button>
142+
</div>
143+
`,
144+
setupConsoleInterceptor( "xon" )
145+
],
146+
({ get }) => {
147+
get( "#xon" ).click()
148+
get( "#errors" ).should(haveText('true'))
149+
},
150+
true
151+
)
152+
153+
test('x-data syntax error',
154+
[html`
155+
<div id="xdata" x-data="{ label: 'Hello' }aaa">
156+
</div>
157+
`,
158+
setupConsoleInterceptor( "xdata" )
159+
],
160+
assertConsoleInterceptorHadErrorWithCorrectElement(),
161+
true
162+
)
163+
164+
test('if statement syntax error',
165+
[html`
166+
<div x-data="{ label: 'Hello' }">
167+
<div id="xtext" x-text="if( false { label } else { 'bye' }"></div>
168+
</div>
169+
`,
170+
setupConsoleInterceptor( "xtext" )
171+
],
172+
assertConsoleInterceptorHadErrorWithCorrectElement(),
173+
true
174+
)
175+
176+
test('x-data with reference error and multiple errors',
177+
[html`
178+
<div id="xdata" x-data="{ items : [ {v:'one'},{v:'two'}], replaceItems }">
179+
<template id="xtext" x-for="item in items">
180+
<span x-text="item.v"></span>
181+
</template>
182+
</div>
183+
`,
184+
setupConsoleInterceptor( "xdata", "xtext" )
185+
],
186+
assertConsoleInterceptorHadErrorWithCorrectElement(),
187+
true
188+
)
189+
190+
test('evaluation with syntax error',
191+
[html`
192+
<div x-data="{value: ''}">
193+
<template id="xif" x-if="value ==== ''">
194+
<span>Words</span>
195+
</template>
196+
</div>
197+
`,
198+
setupConsoleInterceptor( "xif" )
199+
],
200+
assertConsoleInterceptorHadErrorWithCorrectElement(),
201+
true
202+
)

‎tests/cypress/integration/store.spec.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ test('store\'s "this" context is reactive for init function',
7070
[html`
7171
<div x-data>
7272
<span x-text="$store.test.count"></span>
73-
<button @click="$store.test.increment()" id="button">increment</button>
73+
<button id="button">increment</button>
7474
</div>
7575
`,
7676
`

0 commit comments

Comments
 (0)