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

Make identifiers mutable #887

Open
4 tasks
mweststrate opened this issue Jun 19, 2018 · 9 comments
Open
4 tasks

Make identifiers mutable #887

mweststrate opened this issue Jun 19, 2018 · 9 comments
Labels
brainstorming/wild idea Brainstorming / Wild idea

Comments

@mweststrate
Copy link
Member

Mutating identifiers should be an exception, but potentially it would be nice if it was possible to make sure that we can swap client side identifiers for server side generated ones. Should

  • update identifier cache
  • update snapshot of all things referring to the identifier
  • not affect reconciliation
  • new identifier should not be taken already
@mweststrate mweststrate added brainstorming/wild idea Brainstorming / Wild idea breaking change Breaking change labels Jun 19, 2018
@mweststrate mweststrate added this to the 3.0 milestone Jun 19, 2018
@mweststrate mweststrate removed the breaking change Breaking change label Jul 2, 2018
@mweststrate mweststrate removed this from the 3.0 milestone Jul 2, 2018
@broncha
Copy link

broncha commented Sep 26, 2018

Thinking about it a bit... This would open up a possibility to support offline capabilities. A model can be assigned a transient ID while its being synced to the backend. It could very well be added to the store, the user would have an instant feedback, and still show some indication that the model has not synced yet. And when it has an ID from the backend it would update the references and clean up.
I could look into this more. Thanks for mobx and mst :)

@broncha
Copy link

broncha commented Oct 4, 2018

@mweststrate did you have a try on this?
Heres what I have tried

identifier.ts

    reconcile(current: INode, newValue: string) {
        if (current.storedValue !== newValue && current.parent) {
            current.parent!.updateIdentifier(newValue)
        }
        return super.reconcile(current, newValue)
    }

object-node.ts

    updateIdentifier(newIdentifier: string) {
        this.identifier = newIdentifier
        this.root.identifierCache!.changeIdentifier(newIdentifier, this)
    }

identifier-cache.ts

    changeIdentifier(newIdentifier: string, node: ObjectNode) {
        const identifier = node.identifier!

        if (!this.cache.has(newIdentifier)) {
            this.cache.set(newIdentifier, observable.array<ObjectNode>([], mobxShallow))
        }

        const set = this.cache.get(newIdentifier)!
        if (set.indexOf(node) !== -1) fail(`Already registered`)
        set.push(node)

        const prevSet = this.cache.get(identifier)!
        if (prevSet.indexOf(node) == -1) fail(`Already removed`)
        prevSet.remove(node)
    }

Heres my test

test("references are updated when identifier change", () => {
    const values: number[] = []
    const Book = types.model({ id: types.identifier, price: types.number })
    const BookEntry = types.model({ book: types.reference(Book) }).views(self => ({
        get price() {
            return self.book.price * 2
        }
    }))
    const Store = types.model({
        books: types.array(Book),
        entries: types.optional(types.array(BookEntry), [])
    })

    const s = Store.create({ books: [{ id: "1", price: 3 }] })
    unprotect(s)

    s.books.push(Book.create({ id: "3", price: 2 }))

    s.entries.push(BookEntry.create({ book: "3" }))
    expect(s.entries[0].book.price).toBe(2)

    s.books[0].id = "6"
    s.entries.push(BookEntry.create({ book: "6" }))
    expect(s.entries[1].book.price).toBe(2)
})

It throws an exception at s.entries.push(BookEntry.create({ book: "6" })) because the cache update operation has not completed yet.
It seems direct assign to identifiers need to prevented.

@terrysahaidak
Copy link

Any update on this or workaround?
I have an optimistic update in my application and wonder if I could just reassign a new ID but not removing and adding a new item from the server.

@Davidhidalgo
Copy link

This is absolutely necessary for offline capabilities. I'll follow this thread closely and I'll try to come up with ideas. The approach of @broncha seems interesting.

@fullofcaffeine
Copy link

Any workarounds? I'd like to add offline capabilities to a MST app of mine but I'm a bit lost.

@Grolaf
Copy link

Grolaf commented Apr 16, 2024

Hey ! Some time elapsed since the last update in this post. Do someone find out a possible workaround ?

Maybe for further explanations if it was needed, this feature would be really interesting, as the creator of the post said, for server-side generated IDs.

If implemented, this feature will allow models to define a "pushToBackend" action (callable for example with myModelInstance.pushToBackend()) , which could update the instance by itself, preserving the developer from managing the API return himself/herself. This will save a drastic amount of time and technical complexity...

Is this breaking the philosophy of MST ?

Thanks for those who will answer !

@coolsoftwaretyler
Copy link
Collaborator

Hi @Grolaf - I don't know of anyone who is working on this particular feature. I haven't thought much about it, myself. If it's something you're very interested in, I'd be happy to see if we can make a plan and maybe put together a PR.

I don't know if this breaks any MST philosophy, but I also don't know yet if it doesn't, either, haha.

All that to say, I don't have a strong opinion one way or another, I don't think much work is happening on this. Would be open to starting work on it, but I'd want to learn more about the use cases and go about it quite carefully.

Sorry it's not a more interesting answer!

@Grolaf
Copy link

Grolaf commented Apr 17, 2024

Thank you for answering @coolsoftwaretyler ! I would be happy to do it, but since I don't know about MST code yet, it will take me some time to dive in. I'll do it on my free time and probably try some things. Anyway, I think that I have time as this post is open since 2018 😄 .

@coolsoftwaretyler
Copy link
Collaborator

Sounds good! As you make progress will you please respond here so we can coordinate? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
brainstorming/wild idea Brainstorming / Wild idea
Projects
None yet
Development

No branches or pull requests

7 participants