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

Fix reactive validation for subschemas #360

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

lynchem
Copy link

@lynchem lynchem commented Feb 21, 2020

Fixes #359.

It recurses through the schema and adds trackers deps for each one. I wrote this debugging some of our use cases so not sure it's 100% safe. There are no tests that test if the tracker changed function gets called AFAIK, let alone for subschemas. I'm not sure how to go about writing those even. If you think the code is good and have any ideas about how to setup the tests let me know and I'll try create them.

@aldeed
Copy link
Collaborator

aldeed commented Mar 24, 2020

I've looked at this briefly but I'm not sure it's the best way. Somehow depending on the whole subschema and bubbling that up would be more ideal since that's generally how I try to handle subschema stuff. Basically let the subschema handle as much as possible.

One specific change would be to use subschema._deps[key], which should already be there if Tracker was passed into the subschema, which it should be. That way there aren't duplicate reactive dep instances created. I'm not sure if aliasing that to this._deps with the full key is best, or if the reactive functions should instead be updated to drill down into subschemas. Need to think about this a bit more.

@leobastiani
Copy link

Just a friendly reminder here, I came across the same error

@aldeed aldeed changed the base branch from master to main July 30, 2020 14:43
@znewsham
Copy link

znewsham commented Apr 8, 2021

@aldeed I fixed this bug by forcing the creation of a dependency when calling keyErrorMessage

  keyErrorMessage(key) {
    if (!Object.prototype.hasOwnProperty.call(this._deps, key)) {
      this._deps[key] = new Tracker.Dependency();
    }
    const genericKey = MongoObject.makeKeyGeneric(key);
    if (!Object.prototype.hasOwnProperty.call(this._deps, genericKey)) {
      this._deps[genericKey] = new Tracker.Dependency();
    }
    return super.keyErrorMessage(key);
  }

so if you ever ask for the error we reactively depend on it. Would be fairly easy to add this change to ValidationContext

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.

Validation Errors not showing for sub schemas
4 participants