Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: fix(document): allow validation of moddelless documents on node side #8289

Conversation

captaincaius
Copy link
Contributor

Hi - this PR is incomplete because it lacks tests, docs, etc. because I wanted to see if you're okay with this approach first. To me it improves DX and is also a little more maintainable b/c it makes the code paths the same on the client-side and server-side with respect to non-persistent documents.

If you're okay with this direction, please let me know if you have a better idea for a name than "ephemodel" (ephemeral model) :-/.

Fix #8237, fix #8272, and improve DX by making ephemeral models similar
to normal ones

Summary

This fixes modelless document validation on the server-side by sharing its exact same code path with that of browser-side documents.

Browser-side syntax is currently broken on the server-side, so making models for DTO-type sanitizers can currently only be done by making a REAL model.

Examples

const Ephey = goosey.ephemodel(new goosey.Schema({hair: Number}));
const dockey = new Ephey({hair: 'red', eyes: 'green'});
console.log(dockey.validateSync());
console.log(dockey.toObject());

...prints...

{ ValidationError: Validation failed: hair: Cast to Number failed for value "red" at path "hair"
...and...
{}

Fix Automattic#8237, fix Automattic#8272, and improve DX by making ephemeral models similar
to normal ones
@vkarpov15
Copy link
Collaborator

I'm not sure I can support this approach because I really don't want to import browser document into node code. If the only issue is what you saw in #8272, that's a pretty easy fix.

@captaincaius
Copy link
Contributor Author

Thanks for the quick review.

Okay... I had a hunch you'd say that, so I'm glad I didn't get too elaborate with it. I'll close this.

FWIW, I was going to rename browserDocument because a databaseless document is useful beyond the browser, but never mind.

Thanks again.

@vkarpov15
Copy link
Collaborator

Yeah 'databaselessDocument' is overkill since the Document class is databaseless anyway :) The reason why mongoose's Model class inherits from the Document class is that Document is intended to hold all the db-independent stuff: validation, etc. Model has everything that actually requires a database connection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: validateSync() fails on modelless documents fature/bug: databaseless facade on server-side mongoose
2 participants