adopted-ember-addons / ember-changeset-validations

Validations for ember-changeset
http://bit.ly/ember-changeset-demo
Other
218 stars 100 forks source link

Error Importing Validator Class #260

Open topbloc-beiswenger opened 4 years ago

topbloc-beiswenger commented 4 years ago

Using version 3.8.0 of both ember-changeset and ember-changeset-validations, I'm getting an error when importing the Validation class for use in a class-based custom validator: Could not find module ember-changeset-validations/validator. I have imported the Validator class exactly as the docs say:

import Validator from 'ember-changeset-validations/validator';

export default class MyValidator extends Validator {}

I can't seem to find any Validator class within the source code and the tests for class-based validators don't seem to extend the Validator class as well.

Removing extends Validator (like in the tests) prevents the import error from appearing, however, the validations do not run.

KalachevDev commented 4 years ago

I've got the same issue:

ember-source: 3.19.0
ember-changeset: 3.8.1
ember-changeset-validators: 3.8.0
nickschot commented 4 years ago

This is probably because there is no actual class, it is just a type definition as far as I can see. @snewcomer we should probably add a dummy class?

The docs regarding class based validators could also use some extra love for actual usage. I'm currently wrapping the class in a function so using them is identical to that of normal validators.

nickschot commented 4 years ago

Also I can't get it to work at all 🤔 There also don't seem to be tests present (in neither of the repositories) which check if injection of for example a service even works.

snewcomer commented 4 years ago

https://github.com/poteto/ember-changeset/pull/520/files#diff-f721c38c4110379388cab513565fec56R1853

Hi all! I updated the README. It can be a plain class I assumed. Is this not the case?

nickschot commented 4 years ago

https://github.com/poteto/ember-changeset/pull/520/files#diff-f721c38c4110379388cab513565fec56R1853

Hi all! I updated the README. It can be a plain class I assumed. Is this not the case?

I have not fully investigated, but I think they need to be registered with the container one way or another. If you try to inject a service to a bare class it currently errors.

snewcomer commented 4 years ago

I'm not sure we know how to lookup your class from the container. Our changeset is outside the purview of Ember and it's DI system. It would be nice for us to register those validators but we aren't a Service/Controller/etc (can't call getOwner(this)).

I imagine you might be able to register your validators on boot? Or is there something else I am missing?

nickschot commented 4 years ago

@snewcomer I haven't done this before in Ember but I think this is the relevant bits we'll need https://guides.emberjs.com/release/applications/dependency-injection/#toc_factory-registrations

snewcomer commented 4 years ago

We would need a standard folder structure for us to know/register everything in validators/ or something like that. Otherwise we are blind to the application container...

nickschot commented 4 years ago

We would need a standard folder structure for us to know/register everything in validators/ or something like that. Otherwise we are blind to the application container...

Definitely. Don't we already have that though? The cli generators put stuff in validations and validators folders.

snewcomer commented 4 years ago

I think the question becomes - We can certainly register all files in a validators folder (maybe factoryFor) but when we go to look them up, we need something to call getOwner with. We have our helpers but a changeset can be instantiated on its own as well.

topbloc-beiswenger commented 4 years ago

I also found out that class-based validators are not working when using a nested changeset structure, for example

const validations = {
  person: {
    firstName: validatePresence(true)
    lastName: new CustomValidator() // this will not be called
  }
}

This does, however, work when using functional validators. Personally, I think this is an inconsistency and nested changesets should be able to have class-based validators.

topbloc-beiswenger commented 4 years ago

Also if it's currently not possible to inject a service into a class-based validator, should we at least remove @service ajax from the example in the README?

mcfiredrill commented 3 years ago

I haven't had any luck getting a class based validator to work according to the README instructions either.