From 4aa660d4c41892b70c2c5bb5f80a563aa51e24d2 Mon Sep 17 00:00:00 2001 From: Hardik Pithva Date: Tue, 12 Nov 2024 13:25:07 +0100 Subject: [PATCH] fix(form-core): allow to use '_$isValidator$' instead of instanceof check (supporting multiple versions of Validator) * fix: disable dot notation to avoid the renaming for the prop during build/minification * test: add check for _$isValidator$ * chore: cleanup, docs, and add prop on ctor to be more unobtrusive * chore: changeset * fix(form-core): make use of to check validator instance --------- Co-authored-by: Thijs Louisse --- .changeset/chilled-games-care.md | 6 ++++++ package-lock.json | 2 +- .../form-core/src/FormControlMixin.js | 2 +- .../form-core/src/validate/ValidateMixin.js | 10 ++++++++-- .../form-core/src/validate/Validator.js | 20 ++++++++++++++++--- .../form-core/test/validate/Validator.test.js | 8 ++++++++ 6 files changed, 41 insertions(+), 7 deletions(-) create mode 100644 .changeset/chilled-games-care.md diff --git a/.changeset/chilled-games-care.md b/.changeset/chilled-games-care.md new file mode 100644 index 0000000000..9bd425417f --- /dev/null +++ b/.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. diff --git a/package-lock.json b/package-lock.json index cfe83927db..cd2379994a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -20914,7 +20914,7 @@ }, "packages/ui": { "name": "@lion/ui", - "version": "0.5.5", + "version": "0.5.6", "license": "MIT", "dependencies": { "@bundled-es-modules/message-format": "^6.0.4", diff --git a/packages/ui/components/form-core/src/FormControlMixin.js b/packages/ui/components/form-core/src/FormControlMixin.js index 7eda6888cf..5007378113 100644 --- a/packages/ui/components/form-core/src/FormControlMixin.js +++ b/packages/ui/components/form-core/src/FormControlMixin.js @@ -277,7 +277,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')) { diff --git a/packages/ui/components/form-core/src/validate/ValidateMixin.js b/packages/ui/components/form-core/src/validate/ValidateMixin.js index d680c10751..fd7242ee5d 100644 --- a/packages/ui/components/form-core/src/validate/ValidateMixin.js +++ b/packages/ui/components/form-core/src/validate/ValidateMixin.js @@ -10,6 +10,7 @@ import { SyncUpdatableMixin } from '../utils/SyncUpdatableMixin.js'; import { LionValidationFeedback } from './LionValidationFeedback.js'; import { ResultValidator as MetaValidator } from './ResultValidator.js'; import { Unparseable } from './Unparseable.js'; +// eslint-disable-next-line no-unused-vars import { Validator } from './Validator.js'; import { Required } from './validators/Required.js'; import { FormControlMixin } from '../FormControlMixin.js'; @@ -460,7 +461,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 +688,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".`; diff --git a/packages/ui/components/form-core/src/validate/Validator.js b/packages/ui/components/form-core/src/validate/Validator.js index 243058f9b5..bf4c3b5d6f 100644 --- a/packages/ui/components/form-core/src/validate/Validator.js +++ b/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, diff --git a/packages/ui/components/form-core/test/validate/Validator.test.js b/packages/ui/components/form-core/test/validate/Validator.test.js index 5af64d7f61..1cb18dabb6 100644 --- a/packages/ui/components/form-core/test/validate/Validator.test.js +++ b/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');