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

Updating Instance Properties Doesn't Save #110

Open
CatGuardian opened this issue Jan 1, 2018 · 2 comments
Open

Updating Instance Properties Doesn't Save #110

CatGuardian opened this issue Jan 1, 2018 · 2 comments
Assignees
Labels
question This issue is a question about the usage or behaviour of the library
Milestone

Comments

@CatGuardian
Copy link

Installed version: 8.0.0-alpha.7

The problem is that interacting with an Instance that is not created using the corresponding Model doesn't bind the Instance's properties and doesn't save them.

For example:

   // User extends Instance
  // db is an instance of a subclass of Core.
  // db.Users is a Model

   const user = new User(db.Users, {
      name: 'First User',
    });

    user.name = 'changed name before saved';

    const savedUser = await user.save(); // Actually ends up saving { name: 'First User' } to database.

    const retrievedUser = await db.Users.get();

    console.log(`_id: ${retrievedUser._id}   name: ${retrievedUser.name}`);
    // _id: 5a4ac39959d5d83858a0910d   name: First User

But it should end up saving the name as 'changed name before saved'

Right?

@notheotherben
Copy link
Member

Hi @CatGuardian,

You are absolutely correct, however what's happening is that you're using the raw, unprocessed, model before it has been prepared by Iridium - leading to the issues you're seeing.

Before I go into why this is happening, the fix is to replace new User(...) with new db.Users.Instance(...) at which point everything should work correctly (I'll push a test case for this to the repository for you to look at if you are interested).

As for why this happens, you'll perhaps get some insight by looking at the ModelSpecificInstance.ts file. Essentially it creates a pseudo class for a model which proceeds to map each of the schema properties, as top-level object fields, to the underlying document representation for a given Iridium.Instance. This approach enables us to do lazy evaluation of transforms while maintaining V8's virtual class support and avoiding the need for de-optimization which would otherwise be present if you built this dynamically on each request/for each document.

Without this model specific instance proxy, you don't have the field mapping - meaning that your assignment to user.name creates a new name field on your user instance, without actually updating the underlying user._modified.name field or running any transforms. Similarly, if you attempted to access user.name prior to its assignment, it would not retrieve First User, instead returning undefined.

I hope that clears things up for you, please feel free to ask if you have any other questions.

@notheotherben notheotherben added the question This issue is a question about the usage or behaviour of the library label Jan 13, 2018
@notheotherben notheotherben self-assigned this Jan 13, 2018
@CatGuardian
Copy link
Author

CatGuardian commented Jan 20, 2018

Thank you for the response.

Yes that makes sense and clears things up for me.

The thing is, is that I am trying to have my database layer in its own separate npm package. That way any of my projects can add a dependency for my 'my-db' package and start using it. So ideally I was hoping to only export the various Instance classes so I could import them individually as each consumer file needed instead of getting the entire database because certain files won't need to concern themselves with certain Instances/Models.

It does seem a bit awkward to have to go through a roundabout way to instantiate an Instance class when we already define the Instance class as its own class. Meaning I think programmers would much rather do 'new User(...)' than 'new db.Users.Instance(...)'. But if it simply isn't possible to do what I'm asking then I guess I will have to adapt.

Maybe in theory I can do something like this:

class MyDatabase extends Core {
    Houses = new Model<HouseDocument, House>(this, House);
}

var myDb = new MyDatabase({ database: 'houses_test' });

export const HouseModel = myDb.Houses;

And just interact with HouseModel to create proper instances.

And your test case did help. Thank you so much for your time.

It would be nice to see something addressing my use case I asked about up front in the ReadMe portion of this repo. Because the only way mentioned to create a record in the DB is by using the myDb.Houses.insert(...) method.

@notheotherben notheotherben added this to the Version 8 milestone Mar 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question This issue is a question about the usage or behaviour of the library
Projects
None yet
Development

No branches or pull requests

2 participants