fix(select): insensitive multiple.bind order (#1727)

resolves #1724 [skip ci]
aurelia · Apr 5, 2023 · c8d912f · c8d912f
1 parent 48e3f40
commit c8d912f
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 5 deletions.
diff --git a/packages/__tests__/3-runtime-html/select-value-observer.spec.ts b/packages/__tests__/3-runtime-html/select-value-observer.spec.ts
@@ -298,4 +298,30 @@ describe('3-runtime-html/select-value-observer.spec.ts', function () {
 
     await tearDown();
   });
+
+  describe('multiple and value binding order gh #1724 https://github.com/aurelia/aurelia/issues/1724', function () {
+    it('disregards order in simple case', function () {
+      createFixture(
+        `<select value.bind="[]" multiple.bind="true">`,
+      );
+    });
+
+    it('disregard order when there are more attributes in between', function () {
+      createFixture(
+        `<select value.bind="[]" id.bind="a" multiple.bind="true">`,
+      );
+    });
+
+    it('disregard order when there are more attributes at the start', function () {
+      createFixture(
+        `<select id.bind="a" value.bind="[]" multiple.bind="true">`,
+      );
+    });
+
+    it('disregard order when there are more attributes at the end', function () {
+      createFixture(
+        `<select value.bind="[]" multiple.bind="true" id.bind="a">`,
+      );
+    });
+  });
 });
diff --git a/packages/__tests__/esbuild.dev.cjs b/packages/__tests__/esbuild.dev.cjs
@@ -64,7 +64,7 @@ esbuild.context({
     }
   ]
 }).then((ctx) => {
-  if (/^true/.test(process.env.NODE_ENV)) {
+  if (/^true/.test(process.env.DEV_MODE)) {
     ctx.watch();
   }
 }).catch(err => {

diff --git a/packages/runtime-html/src/compiler/template-compiler.ts b/packages/runtime-html/src/compiler/template-compiler.ts
@@ -20,6 +20,7 @@ import {
   ITemplateCompiler,
   PropertyBindingInstruction,
   SpreadElementPropBindingInstruction,
+  InstructionType,
 } from '../renderer';
 import { IPlatform } from '../platform';
 import { Bindable, BindableDefinition } from '../bindable';
@@ -646,6 +647,9 @@ export class TemplateCompiler implements ITemplateCompiler {
     let attrName: string;
     let attrValue: string;
     let attrSyntax: AttrSyntax;
+    /**
+     * A list of plain attribute bindings/interpolation bindings
+     */
     let plainAttrInstructions: IInstruction[] | undefined;
     let elBindableInstructions: IInstruction[] | undefined;
     let attrDef: CustomAttributeDefinition | null = null;
@@ -920,7 +924,7 @@ export class TemplateCompiler implements ITemplateCompiler {
 
     resetCommandBuildInfo();
 
-    if (this._shouldReorderAttrs(el) && plainAttrInstructions != null && plainAttrInstructions.length > 1) {
+    if (this._shouldReorderAttrs(el, plainAttrInstructions) && plainAttrInstructions != null && plainAttrInstructions.length > 1) {
       this._reorder(el, plainAttrInstructions);
     }
 
@@ -1575,8 +1579,13 @@ export class TemplateCompiler implements ITemplateCompiler {
   }
 
   /** @internal */
-  private _shouldReorderAttrs(el: Element): boolean {
-    return el.nodeName === 'INPUT' && orderSensitiveInputType[(el as HTMLInputElement).type] === 1;
+  private _shouldReorderAttrs(el: Element, instructions?: IInstruction[]): boolean | undefined {
+    const nodeName = el.nodeName;
+    return nodeName === 'INPUT' && orderSensitiveInputType[(el as HTMLInputElement).type] === 1
+      || nodeName === 'SELECT' && (
+        (el as HTMLSelectElement).hasAttribute('multiple')
+        || instructions?.some(i => i.type === InstructionType.propertyBinding && (i as PropertyBindingInstruction | InterpolationInstruction).to === 'multiple')
+      );
   }
 
   /** @internal */
@@ -1608,6 +1617,32 @@ export class TemplateCompiler implements ITemplateCompiler {
         if (checkedIndex !== void 0 && modelOrValueOrMatcherIndex !== void 0 && checkedIndex < modelOrValueOrMatcherIndex) {
           [_instructions[modelOrValueOrMatcherIndex], _instructions[checkedIndex]] = [_instructions[checkedIndex], _instructions[modelOrValueOrMatcherIndex]];
         }
+        break;
+      }
+      case 'SELECT': {
+        const _instructions = instructions as (PropertyBindingInstruction | InterpolationInstruction)[];
+        let valueIndex = 0;
+        let multipleIndex = 0;
+        // a variable to stop the loop as soon as we find both value & multiple binding indices
+        let found = 0;
+        let instruction: PropertyBindingInstruction | InterpolationInstruction;
+        // swap the order of multiple and value bindings
+        for (let i = 0; i < _instructions.length && found < 2; ++i) {
+          instruction = _instructions[i];
+          switch (instruction.to) {
+            case 'multiple':
+              multipleIndex = i;
+              found++;
+              break;
+            case 'value':
+              valueIndex = i;
+              found++;
+              break;
+          }
+          if (found === 2 && valueIndex < multipleIndex) {
+            [_instructions[multipleIndex], _instructions[valueIndex]] = [_instructions[valueIndex], _instructions[multipleIndex]];
+          }
+        }
       }
     }
   }

diff --git a/packages/runtime-html/src/observation/select-value-observer.ts b/packages/runtime-html/src/observation/select-value-observer.ts
@@ -265,7 +265,7 @@ export class SelectValueObserver implements INodeObserver {
     if (array != null) {
       if (!this._el.multiple) {
         if (__DEV__)
-          throw createError(`AUR0654: Only null or Array instances can be bound to a multi-select.`);
+          throw createError(`AUR0654: array values can only be bound to a multi-select.`);
         else
           throw createError(`AUR0654`);
       }