Skip to content

Commit

Permalink
fix(form-core): allow to use '_$isValidator$' instead of instanceof c…
Browse files Browse the repository at this point in the history
…heck (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 <[email protected]>
  • Loading branch information
hardikpthv and tlouisse authored Nov 12, 2024
1 parent 1a485bd commit 4aa660d
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 7 deletions.
6 changes: 6 additions & 0 deletions .changeset/chilled-games-care.md
Original file line number Diff line number Diff line change
@@ -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.
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/ui/components/form-core/src/FormControlMixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')) {
Expand Down
10 changes: 8 additions & 2 deletions packages/ui/components/form-core/src/validate/ValidateMixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 }];
}
Expand Down Expand Up @@ -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".`;
Expand Down
20 changes: 17 additions & 3 deletions packages/ui/components/form-core/src/validate/Validator.js
Original file line number Diff line number Diff line change
@@ -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
*/

Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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');
Expand Down

0 comments on commit 4aa660d

Please sign in to comment.