fix(form-core): allow to use '_$isValidator$' instead of instanceof c…

…heck (supporting multiple versions of Validator)

* fix(form-core): make use of `validatorName` and `type` to check validator instance

* fix(form-core): check for nullability for disabled prop

* fix: add `_$isValidator$` property to improve the check

* fix: disable dot notation to avoid the renaming for the prop during build/minification

* fix: disable dot notation to avoid the renaming for the prop during build/minification

* test: add check for _$isValidator$

* chore: catch edge cases in cem script

* chore: cleanup, docs, and add prop on ctor to be more unobtrusive

* chore: changeset

---------

Co-authored-by: Thijs Louisse <[email protected]>

.changeset/chilled-games-care.md

@@ -0,0 +1,6 @@
+---
+'@lion/ui': patch
+---
+
+Add unique ['_$isValidator$'] marker to identify Validator instances across different lion versions.
+It's meant to be used as a replacement for an `instanceof` check.

packages/ui/components/form-core/src/FormControlMixin.js

@@ -274,7 +274,7 @@ const FormControlMixinImplementation = superclass =>
       super.updated(changedProperties);
 
       if (changedProperties.has('disabled')) {
-        this._inputNode?.setAttribute('aria-disabled', this.disabled.toString());
+        this._inputNode?.setAttribute('aria-disabled', `${Boolean(this.disabled)}`);
       }
 
       if (changedProperties.has('_ariaLabelledNodes')) {

packages/ui/components/form-core/src/validate/ValidateMixin.js

@@ -10,10 +10,10 @@ import { SyncUpdatableMixin } from '../utils/SyncUpdatableMixin.js';
 import { LionValidationFeedback } from './LionValidationFeedback.js';
 import { ResultValidator as MetaValidator } from './ResultValidator.js';
 import { Unparseable } from './Unparseable.js';
-import { Validator } from './Validator.js';
 import { Required } from './validators/Required.js';
 import { FormControlMixin } from '../FormControlMixin.js';
-
+// eslint-disable-next-line no-unused-vars
+import { Validator } from './Validator.js';
 // TODO: [v1] make all @readOnly => @readonly and actually make sure those values cannot be set
 
 /**
@@ -460,7 +460,9 @@ export const ValidateMixinImplementation = superclass =>
 
       if (isEmpty) {
         const hasSingleValue = !(/** @type {*  & ValidateHost} */ (this)._isFormOrFieldset);
-        const requiredValidator = this._allValidators.find(v => v instanceof Required);
+        const requiredValidator = this._allValidators.find(
+          v => /** @type {typeof Validator} */ (v.constructor)?.validatorName === 'Required',
+        );
         if (requiredValidator) {
           this.__syncValidationResult = [{ validator: requiredValidator, outcome: true }];
         }
@@ -685,7 +687,10 @@ export const ValidateMixinImplementation = superclass =>
       }
 
       for (const validatorToSetup of this._allValidators) {
-        if (!(validatorToSetup instanceof Validator)) {
+        // disable dot notation to avoid the renaming for the prop during build/minification
+        const validatorCtor = /** @type {typeof Validator}  */ (validatorToSetup.constructor);
+        // eslint-disable-next-line dot-notation
+        if (validatorCtor['_$isValidator$'] === undefined) {
           // throws in constructor are not visible to end user so we do both
           const errorType = Array.isArray(validatorToSetup) ? 'array' : typeof validatorToSetup;
           const errorMessage = `Validators array only accepts class instances of Validator. Type "${errorType}" found. This may be caused by having multiple installations of "@lion/ui/form-core.js".`;

packages/ui/components/form-core/src/validate/Validator.js

@@ -1,10 +1,10 @@
 /**
  * @typedef {import('../../types/validate/index.js').FeedbackMessageData} FeedbackMessageData
- * @typedef {import('../../types/validate/index.js').ValidatorParam} ValidatorParam
- * @typedef {import('../../types/validate/index.js').ValidatorConfig} ValidatorConfig
  * @typedef {import('../../types/validate/index.js').ValidatorOutcome} ValidatorOutcome
- * @typedef {import('../../types/validate/index.js').ValidatorName} ValidatorName
+ * @typedef {import('../../types/validate/index.js').ValidatorConfig} ValidatorConfig
+ * @typedef {import('../../types/validate/index.js').ValidatorParam} ValidatorParam
  * @typedef {import('../../types/validate/index.js').ValidationType} ValidationType
+ * @typedef {import('../../types/validate/index.js').ValidatorName} ValidatorName
  * @typedef {import('../FormControlMixin.js').FormControlHost} FormControlHost
  */
 
@@ -32,8 +32,22 @@ export class Validator extends EventTarget {
      * @type {ValidationType}
      */
     this.type = config?.type || 'error';
+    /**
+     * Disable dot notation to avoid the renaming for the prop during build/minification
+     */
   }
 
+  /**
+   * This unique marker allows us to identify Validator instances across different lion versions.
+   * It's meant to be used as a replacement for an instanceof check.
+   *
+   * N.B. it's important to keep the bracket notation, as `static _$isValidator$ = true;` will
+   * be renamed during minification and that leads to different names across different
+   * lion versions.
+   * @type {boolean}
+   */
+  static ['_$isValidator$'] = true;
+
   /**
    * The name under which validation results get registered. For convenience and predictability, this
    * should always be the same as the constructor name (since it will be obfuscated in js builds,

packages/ui/components/form-core/test/validate/Validator.test.js

@@ -1,3 +1,4 @@
+/* eslint-disable dot-notation */
 import { LitElement } from 'lit';
 import { defineCE, expect, fixture, html, unsafeStatic } from '@open-wc/testing';
 import sinon from 'sinon';
@@ -187,6 +188,13 @@ describe('Validator', () => {
     expect(disconnectSpy.calledWith(el)).to.equal(true);
   });
 
+  it('adds static ["_$isValidator$"] property as a marker to identify the Validator class across different lion versions (as instanceof cannot be used)', async () => {
+    const myValidator = new Validator();
+
+    expect(myValidator.constructor['_$isValidator$']).to.exist;
+    expect(myValidator.constructor['_$isValidator$']).to.be.true;
+  });
+
   describe('Types', () => {
     it('has type "error" by default', async () => {
       expect(new Validator().type).to.equal('error');

packages/ui/scripts/cem.js

@@ -39,10 +39,10 @@ for (const [, module] of moduleGraph.modules) {
 
 /** Exclude information inherited from these mixins, they're generally not useful for public api */
 const inheritanceDenyList = [
-  'LocalizeMixin',
   'ScopedElementsMixin',
-  'SlotMixin',
   'SyncUpdatableMixin',
+  'LocalizeMixin',
+  'SlotMixin',
 ];
 
 const cem = create({
@@ -66,11 +66,11 @@ const cem = create({
                * Set privacy for members based on naming conventions
                */
               for (const m of declaration.members) {
-                if (!m.privacy && !m.name.startsWith('_') && !m.name.startsWith('#')) {
+                if (!m.privacy && !m.name?.startsWith('_') && !m.name?.startsWith('#')) {
                   m.privacy = 'public';
-                } else if (!m.privacy && m.name.startsWith('_')) {
+                } else if (!m.privacy && m.name?.startsWith('_')) {
                   m.privacy = 'protected';
-                } else if (m.name.startsWith('#') || m.name.startsWith('__')) {
+                } else if (m.name?.startsWith('#') || m.name?.startsWith('__')) {
                   m.privacy = 'private';
                 }
               }