Merge branch 'main' into morph-@click

alpinejs · May 10, 2023 · a0a6b74 · a0a6b74
2 parents 2018f5f + 0f17459
commit a0a6b74
Show file tree

Hide file tree

Showing 12 changed files with 165 additions and 22 deletions.
diff --git a/README.md b/README.md
@@ -8,7 +8,7 @@ Stay here for contribution-related information.
 
 > Looking for V2 docs? [here they are](https://github.com/alpinejs/alpine/tree/v2.8.2)
 
-<p align="center"><a href="https://alpinejs.dev/patterns"><img src="/hero.jpg" alt="Alpine Compoenent Patterns"></a></p>
+<p align="center"><a href="https://alpinejs.dev/patterns"><img src="/hero.jpg" alt="Alpine Component Patterns"></a></p>
 
 ## Contribution Guide:
 

diff --git a/packages/alpinejs/src/directives/x-for.js b/packages/alpinejs/src/directives/x-for.js
@@ -158,6 +158,8 @@ function loop(el, iteratorNames, evaluateItems, evaluateKey) {
             let marker = document.createElement('div')
 
             mutateDom(() => {
+                if (! elForSpot) warn(`x-for ":key" is undefined or invalid`, templateEl)
+
                 elForSpot.after(marker)
                 elInSpot.after(elForSpot)
                 elForSpot._x_currentIfEl && elForSpot.after(elForSpot._x_currentIfEl)

diff --git a/packages/alpinejs/src/directives/x-model.js b/packages/alpinejs/src/directives/x-model.js
@@ -46,10 +46,6 @@ directive('model', (el, { modifiers, expression }, { effect, cleanup }) => {
             })
         }
     }
-
-    if (modifiers.includes('fill') && el.hasAttribute('value') && (getValue() === null || getValue() === '')) {
-        setValue(el.value)
-    }
 
     if (typeof expression === 'string' && el.type === 'radio') {
         // Radio buttons only work properly when they share a name attribute.
@@ -73,7 +69,10 @@ directive('model', (el, { modifiers, expression }, { effect, cleanup }) => {
     let removeListener = isCloning ? () => {} : on(el, event, modifiers, (e) => {
         setValue(getInputValue(el, modifiers, e, getValue()))
     })
-
+
+    if (modifiers.includes('fill') && [null, ''].includes(getValue())) {
+        el.dispatchEvent(new Event(event, {}));
+    }
     // Register the listener removal callback on the element, so that
     // in addition to the cleanup function, x-modelable may call it.
     // Also, make this a keyed object if we decide to reintroduce
@@ -134,9 +133,9 @@ function getInputValue(el, modifiers, event, currentValue) {
         // Check for event.detail due to an issue where IE11 handles other events as a CustomEvent.
         // Safari autofill triggers event as CustomEvent and assigns value to target
         // so we return event.target.value instead of event.detail
-        if (event instanceof CustomEvent && event.detail !== undefined) {
-            return typeof event.detail != 'undefined' ? event.detail : event.target.value
-        } else if (el.type === 'checkbox') {
+        if (event instanceof CustomEvent && event.detail !== undefined)
+            return event.detail ?? event.target.value
+        else if (el.type === 'checkbox') {
             // If the data we are binding to is an array, toggle its value inside the array.
             if (Array.isArray(currentValue)) {
                 let newValue = modifiers.includes('number') ? safeParseNumber(event.target.value) : event.target.value

diff --git a/packages/alpinejs/src/directives/x-transition.js b/packages/alpinejs/src/directives/x-transition.js
@@ -7,8 +7,8 @@ import { once } from '../utils/once'
 
 directive('transition', (el, { value, modifiers, expression }, { evaluate }) => {
     if (typeof expression === 'function') expression = evaluate(expression)
-
-    if (! expression) {
+    if (expression === false) return
+    if (!expression || typeof expression === 'boolean') {
         registerTransitionsFromHelper(el, modifiers, value)
     } else {
         registerTransitionsFromClassString(el, expression, value)

diff --git a/packages/alpinejs/src/lifecycle.js b/packages/alpinejs/src/lifecycle.js
@@ -4,7 +4,13 @@ import { dispatch } from './utils/dispatch'
 import { walk } from "./utils/walk"
 import { warn } from './utils/warn'
 
+let started = false
+
 export function start() {
+    if (started) warn('Alpine has already been initialized on this page. Calling Alpine.start() more than once can cause problems.')
+
+    started = true
+
     if (! document.body) warn('Unable to initialize. Trying to load Alpine before `<body>` is available. Did you forget to add `defer` in Alpine\'s `<script>` tag?')
 
     dispatch(document, 'alpine:init')

diff --git a/packages/alpinejs/src/magics.js b/packages/alpinejs/src/magics.js
@@ -11,17 +11,24 @@ export function magic(name, callback) {
 
 export function injectMagics(obj, el) {
     Object.entries(magics).forEach(([name, callback]) => {
-        Object.defineProperty(obj, `$${name}`, {
-            get() {
+        let memoizedUtilities = null;
+        function getUtilities() {
+            if (memoizedUtilities) {
+                return memoizedUtilities;
+            } else {
                 let [utilities, cleanup] = getElementBoundUtilities(el)
 
-                utilities = {interceptor, ...utilities}
+                memoizedUtilities = {interceptor, ...utilities}
 
                 onElRemoved(el, cleanup)
-
-                return callback(el, utilities)
+                return memoizedUtilities;
+            }
+        }
+
+        Object.defineProperty(obj, `$${name}`, {
+            get() {
+                return callback(el, getUtilities());
             },
-
             enumerable: false,
         })
     })

diff --git a/packages/alpinejs/src/utils/bind.js b/packages/alpinejs/src/utils/bind.js
@@ -22,6 +22,14 @@ export default function bind(el, name, value, modifiers = []) {
         case 'class':
             bindClasses(el, value)
             break;
+
+        // 'selected' and 'checked' are special attributes that aren't necessarily
+        // synced with their corresponding properties when updated, so both the 
+        // attribute and property need to be updated when bound.
+        case 'selected':
+        case 'checked':
+            bindAttributeAndProperty(el, name, value)
+            break;
 
         default:
             bindAttribute(el, name, value)
@@ -78,6 +86,11 @@ function bindStyles(el, value) {
     el._x_undoAddedStyles = setStyles(el, value)
 }
 
+function bindAttributeAndProperty(el, name, value) {
+    bindAttribute(el, name, value)
+    setPropertyIfChanged(el, name, value)
+}
+
 function bindAttribute(el, name, value) {
     if ([null, undefined, false].includes(value) && attributeShouldntBePreservedIfFalsy(name)) {
         el.removeAttribute(name)
@@ -94,6 +107,12 @@ function setIfChanged(el, attrName, value) {
     }
 }
 
+function setPropertyIfChanged(el, propName, value) {
+    if (el[propName] !== value) {
+        el[propName] = value
+    }
+}
+
 function updateSelect(el, value) {
     const arrayWrappedValue = [].concat(value).map(value => { return value + '' })
 

diff --git a/tests/cypress/integration/directives/x-bind.spec.js b/tests/cypress/integration/directives/x-bind.spec.js
@@ -1,4 +1,4 @@
-import { beHidden, beVisible, haveText, beChecked, haveAttribute, haveClasses, haveValue, notBeChecked, notHaveAttribute, notHaveClasses, test, html } from '../../utils'
+import { beHidden, beVisible, haveText, beChecked, haveAttribute, haveClasses, haveProperty, haveValue, notBeChecked, notHaveAttribute, notHaveClasses, test, html } from '../../utils';
 
 test('sets attribute bindings on initialize',
     html`
@@ -391,8 +391,8 @@ test('x-bind object syntax event handlers defined as functions receive the event
         <script>
             window.data = () => { return {
                 button: {
-                    ['@click']() {
-                        this.$refs.span.innerText = this.$el.id
+                    ['@click'](event) {
+                        this.$refs.span.innerText = event.currentTarget.id
                     }
                 }
             }}
@@ -410,7 +410,7 @@ test('x-bind object syntax event handlers defined as functions receive the event
     }
 )
 
-test('x-bind object syntax event handlers defined as functions receive the event object as their first argument',
+test('x-bind object syntax event handlers defined as functions receive element bound magics',
     html`
         <script>
             window.data = () => { return {
@@ -452,3 +452,22 @@ test('Can retrieve Alpine bound data with global bound method',
         get('#6').should(haveText('bar'))
     }
 )
+
+test('x-bind updates checked attribute and property after user interaction',
+    html`
+        <div x-data="{ checked: true }">
+            <button @click="checked = !checked">toggle</button>
+            <input type="checkbox" x-bind:checked="checked" @change="checked = $event.target.checked" />
+        </div>
+    `,
+    ({ get }) => {
+        get('input').should(haveAttribute('checked', 'checked'))
+        get('input').should(haveProperty('checked', true))
+        get('input').click()
+        get('input').should(notHaveAttribute('checked'))
+        get('input').should(haveProperty('checked', false))
+        get('button').click()
+        get('input').should(haveAttribute('checked', 'checked'))
+        get('input').should(haveProperty('checked', true))
+    }
+)
diff --git a/tests/cypress/integration/directives/x-for.spec.js b/tests/cypress/integration/directives/x-for.spec.js
@@ -559,3 +559,29 @@ test('renders children using directives injected by x-html correctly',
         get('p:nth-of-type(2) span').should(haveText('bar'))
     }
 )
+
+test('x-for throws descriptive error when key is undefined',
+    html`
+        <div x-data="{ items: [
+            {
+                id: 1,
+                name: 'foo',
+            },
+            {
+                id: 2,
+                name: 'bar',
+            },
+            {
+                id: 3,
+                name: 'baz',
+            },
+        ]}">
+            <template x-for="item in items" :key="item.doesntExist">
+                <span x-text="i"></span>
+            </template>
+        </div>
+    `,
+    ({ get }) => {
+    },
+    true
+)
diff --git a/tests/cypress/integration/directives/x-model.spec.js b/tests/cypress/integration/directives/x-model.spec.js
@@ -151,5 +151,40 @@ test('x-model with fill modifier takes input value on null or empty string',
     }
 )
 
+test('x-model with fill modifier works with select/radio elements',
+    html`
+        <div x-data="{ a: null, b: null, c: null, d: null }">
+            <select x-model.fill="a">
+                <option value="123">123</option>
+                <option value="456" selected>456</option>
+            </select>
+            <select x-model.fill="b" multiple>
+                <option value="123" selected>123</option>
+                <option value="456" selected>456</option>
+            </select>
+        </div>
+    `,
+    ({ get }) => {
+        get('[x-data]').should(haveData('a', '456'));
+        get('[x-data]').should(haveData('b', ['123', '456']));
+    }
+);
+
+test('x-model with fill modifier respects number modifier',
+    html`
+        <div x-data="{ a: null, b: null, c: null, d: null }">
+            <input type="text" x-model.fill.number="a" value="456" / >
+            <select x-model.fill.number="b" multiple>
+                <option value="123" selected>123</option>
+                <option value="456" selected>456</option>
+            </select>
+        </div>
+    `,
+    ({ get }) => {
+        get('[x-data]').should(haveData('a', 456));
+        get('[x-data]').should(haveData('b', [123,456]));
+    }
+);
+
 
 
diff --git a/tests/cypress/integration/directives/x-transition.spec.js b/tests/cypress/integration/directives/x-transition.spec.js
@@ -80,3 +80,31 @@ test('transition:enter in nested x-show visually runs',
         get('h1').should(haveComputedStyle('opacity', '1')) // Eventually opacity will be 1
     }
 )
+
+test(
+    'bound x-transition can handle empty string and true values',
+    html`
+        <script>
+            window.transitions = () => {
+                return {
+                    withEmptyString: {
+                        ["x-transition.opacity"]: "",
+                    },
+                    withBoolean: {
+                        ["x-transition.opacity"]: true,
+                    },
+                };
+            };
+        </script>
+        <div x-data="transitions()">
+            <button x-bind="withEmptyString"></button>
+            <span x-bind="withBoolean">thing</span>
+        </div>
+    `,
+    ({ get }) => 
+        {
+            get('span').should(beVisible())
+            get('span').should(beVisible())
+        }
+
+);
diff --git a/tests/cypress/utils.js b/tests/cypress/utils.js
@@ -87,7 +87,7 @@ function injectHtmlAndBootAlpine(cy, templateAndPotentiallyScripts, callback, pa
     })
 }
 
-export let haveData = (key, value) => ([ el ]) => expect(root(el)._x_dataStack[0][key]).to.equal(value)
+export let haveData = (key, value) => ([el]) => expect(root(el)._x_dataStack[0][key]).to.deep.equal(value);
 
 export let haveFocus = () => el => expect(el).to.have.focus
 
@@ -97,6 +97,8 @@ export let haveAttribute = (name, value) => el => expect(el).to.have.attr(name,
 
 export let notHaveAttribute = (name, value) => el => expect(el).not.to.have.attr(name, value)
 
+export let haveProperty = (name, value) => el => expect(el).to.have.prop(name, value)
+
 export let haveText = text => el => expect(el).to.have.text(text)
 
 export let notHaveText = text => el => expect(el).not.to.have.text(text)