emberjs / ember.js

Ember.js - A JavaScript framework for creating ambitious web applications
https://emberjs.com
MIT License
22.45k stars 4.21k forks source link

`tracked` no longer works with non-decorator style use #18362

Closed trek closed 4 years ago

trek commented 5 years ago

This may possibly be better living at at https://github.com/glimmerjs/glimmer.js but I encountered this issue using tracked with latest Octane.

My current use case is tracking the permission property of window.Notification, which after some chatter with @mmun and @machty in the alumni lounge seems like it should be possible either directly or with some handwaving.

This comment (h/t @mmun) also makes it seem like this is possible:

import { tracked } from '@glimmer/tracking';
tracked(Notification.prototype, 'name'); 

But this errors with The options object passed to tracked() may only contain a 'value' or 'initializer' property, not both coming form here.

We tried doing some of the descriptor setting ourselves

import { tracked } from '@glimmer/tracking';

function track(object, key) {
  let originalDescriptor = Object.getOwnPropertyDescriptor(object, key);
  let trackedDescriptor = tracked(object, key, originalDescriptor);
  Object.defineProperty(object, key, trackedDescriptor);
}

track(window.Notification, "permission");

But this also errors with You attempted to use @tracked on permission, but that element is not a class field. from here

pzuraq commented 5 years ago

That is not a supported method for using tracked(). It can be used with Ember.defineProperty however, similar to computed(), which means that it works with classic class definitions:

const MyClass = EmberObject.extend({
  foo: tracked()
});

Ember.defineProperty(window.Notification, 'permission', tracked());

I'm not sure I would recommend this method, but if you're trying to avoid using a service it may make sense. We are trying to figure out the correct shape of public APIs for autotracking in general, and Ember.defineProperty will probably eventually be removed from the framework (I'd imagine around the same time as EmberObject), so you'd likely have to migrate to something else in the future.

trek commented 5 years ago

@pzuraq gotcha. Is this a reversal of earlier behaviors? https://github.com/glimmerjs/glimmer.js/issues/22#issuecomment-336441371

mmun commented 5 years ago

@pzuraq And the relevant code that allows (allowed? It's confusing having parallel impls :P) it: https://github.com/glimmerjs/glimmer.js/blob/master/packages/@glimmer/tracking/src/tracked.ts#L116.

Regarding your example,

Ember.defineProperty(window.Notification, 'permission', tracked());

That works for some (most?) cases, but it doesn't preserve the existing descriptor if one exists.

pzuraq commented 5 years ago

Ah, Rob's point there is essentially you can call it like a decorator directly, you would probably be able to do something like:

let oldDesc = Object.getOwnPropertyDescriptor(window.Notification, 'permission');
let newDesc = tracked(window.Notification, 'permission', oldDesc);
Object.defineProperty(window.Notification, 'permission', newDesc);

I would definitely caution against this, since decorators will be changing in the future, I think that discussion happened before some of the changes upstream in decorators.

@mmun the implementation for Ember's version currently lives here: https://github.com/emberjs/ember.js/blob/master/packages/@ember/-internals/metal/lib/tracked.ts

That functionality in Glimmer seems odd, unsure why it was added (doesn't seem to be like TS decorators at all), but that was not part of the RFC'd API which is why it wasn't transitioned over to Ember.js. Eventually, the Glimmer.js API will be updated to match the Ember.js one. Definitely think something like this could be added for dynamically defining tracked props, but that'll have to go through RFC too

locks commented 4 years ago

This should be resolved as per the discussion, closing. Thanks for your time everyone!

snewcomer commented 2 years ago

like this could be added for dynamically defining tracked props, but that'll have to go through RFC too

I do have a use case, but solvable. Ideally, I could define the property on the function _statefulFunc that reads from a tracked property on the created StatefulPromise. However, I had to create an intermediate Handler class that is explicitly updated based on the state of the promise.

https://github.com/snewcomer/ember-stateful-promise/blob/1794135b68dba0623870288c0cd8a94f7872c8eb/addon/decorators/stateful-function.js#L5-L25