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

5.x | value.from should reflect over promises when needed #32

Open
rjgotten opened this issue Apr 28, 2020 · 3 comments
Open

5.x | value.from should reflect over promises when needed #32

rjgotten opened this issue Apr 28, 2020 · 3 comments

Comments

@rjgotten
Copy link

rjgotten commented Apr 28, 2020

Given an observable such as value.from( map, "promised.value" ) - internally reflectPromise is not used to make the promised property's result observable, if it returns something promise-like.

This is a major step back from using can.compute.read in older versions of CanJS, which did apply promise reflection transparantly.

One could argue not hooking up the behavior for performance reasons, but in that case I'd argue that it should be possible to look ahead to the next key in the chain ("value" in this case); to note that it's one of the special promise meta-keys; and to then selectively apply the reflectPromise hookup.

As it stands, I'm now writing:

results : {
  value( prop ) {
    const results = value.returnedBy(() => {
      const promise = this.query;

      if ( promise ) {
        reflectPromise( promise );
        const value = Reflect.getKeyValue( promise, "value" );
        return value != null ? value.results : null;
      }

      return null;
    });

    prop.listenTo( results, prop.resolve );
    prop.resolve( results.value );
  }
},

where really, I want to be writing:

results : {
  value( prop ) {
    const results = value.from( this, "query.value.results" );

    prop.listenTo( results, prop.resolve );
    prop.resolve( results.value );
  }
},

This problem is devilishly insidious, as reflectPromise ties itself to the passed promise's prototype, where possible.

This means if another totally unrelated promise that just happens to share its prototype, is accessed in a stache template (where reflectPromise is auto-applied) - then 'magically' things work and you're not aware you've created a submarine bug.

Diagnosing that bug is hard and gets even worse when your code putting promises into stache templates is async and relies on network conditions -- e.g. separate components mounting from separate webpack chunks. At that point things may or may not 'magically' work, depending on non-deterministic order of execution.

@justinbmeyer
Copy link
Contributor

You can probably use can-stache-key

@justinbmeyer
Copy link
Contributor

I’m not fully reading but

This problem is devilishly insidious, as reflectPromise ties itself to the passed promise's prototype, where possible.

That doesn’t strike me as true. We add to the instance, not the prototype.

@rjgotten
Copy link
Author

rjgotten commented Apr 30, 2020

@justinbmeyer
That doesn’t strike me as true. We add to the instance, not the prototype.

The actual state storage is attached per-instance, but the symbols implementing the observability hooks themselves like can.getKeyValue and can.onKeyValue are attached to the prototype for prototype-based promises. (Those symbols then lazily initialize the state for individual instances on first-access.)

https://github.com/canjs/can-reflect-promise/blob/a3e31dcf4b431898e567629e0efe59658e242218/can-reflect-promise.js#L92-L133

You can probably use can-stache-key

Thanks. Will have a look at that.

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

No branches or pull requests

2 participants