Git Product home page Git Product logo

Comments (2)

runspired avatar runspired commented on June 2, 2024

iirc polymorphism in 5.2 has a bunch of broken edge cases, I wouldn't necessarily consider it the definitive version of "Was it supposed to work this way". We fixed it in 5.3 and backported the fix to 4.12. That is assuming of course this is even related to those changes, I suspect its not. It's probably other improvements around identity and simplification of internal timing that then exposes you to the losing end of a race condition here.

Generally speaking, we have never explicitly supported polymorphic find in this way. That said, we did some creative things when we landed the identifier cache in the mid 3.x series to make this scenario work most-but-not-all of the time (it used to work never). This most-of-the-time approach works whenever two polymorphic identities can be detected to belong to the same identity and we have not created a record for the initial identity (it also works some of the time when we have, though thats complicated).

If you want this scenario to work all of the time, then you end up needing to implement the identity generation hooks yourself, and using the context of your app domain to ensure the polymorphic identities are merged into a single identity. There's a bunch of polymorphic scenario tests that may help steer you in the general direction of what to do if it comes to it. This said in general findRecord and preload are two APIs that will almost certainly get you into trouble with polymorphism.

On this line when you preload, this is going to cause the entity identity to be created, and it won't know that company is valid for it.

All this said, the easy way around this is probably just to not use store.findRecord in favor of using the request pattern. After configuring any handlers you need for response normalization:

store.request(findRecord('property', propertyId, {}, { resourcePath: entities/${entityId}/properties }))

If you are making this sort of request a lot you can either create your own builder (there's utils for builders too), or just wrap an existing builder to do the above if that's pretty much what you need.

from data.

runspired avatar runspired commented on June 2, 2024

After digging in more I can give you more insight into why what you are doing breaks in 5.3 vs 5.2.

At the point you initiate the preload request, the store has already realized the entity-1 was company-1 and merged the two identifiers. Then the preload swaps the information for the relationship back to entity-1 generating a new identifier. Basically, because the preload is treated as "cache state", you replace what past payloads told you and end up somewhere else.

This becomes a problem in 5.3 specifically because we had to remove one of the more creative things we'd done to try to enable polymorphism to mostly work even without the consuming app configuring the identity hooks:

// ensure a secondary cache entry for this id for the identifier we do keep
keyOptions.id.set(newId, kept);
// ensure a secondary cache entry for this id for the abandoned identifier's type we do keep
let baseKeyOptions = getTypeIndex(this._cache.types, existingIdentifier.type);
baseKeyOptions.id.set(newId, kept);

These lines were creating a reverse lookup so that if we merged two identifiers (as happens in your case) and later encounter the discarded identifier again (which happens due to the preload data being assigned to an already loaded record in your example) we would recognize that this was a previously merged identifier and return the original.

The trouble is this meant that we couldn't give the identity generation hook the full control it needs for the case where resources are not json values, and this hack for polymorphism was only ever going to work with json that had id specifically as top level field as opposed to say urn or primaryKey or anything else that might be equally valid.

As an aside, none of this would matter if it weren't for the fact that the route and parent route attempt to rerender with the new state you are preloading while the request is still in-flight. This is yet another reason why using requestManager here instead would be beneficial.

I'm considering whether there's some other way to bring back the utility of the reverse lookup without breaking the opaque idea of resources these APIs demand. I'm not sure there is, and I'm also not sure the utility of it given in more modernized usage this problem and many more like it wouldn't be encountered, but if it turns out to not be too hard then its probably ok to do.

from data.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.