[Bug] v3.20.6 introduces breaking change for @dependentKeyCompat

[fran-worley]

🐞 Describe the Bug

Updating from 3.20.5 results in the following error:

Assertion Failed: You attempted to use @dependentKeyCompat on a property that already has been decorated with either 
@computed or @tracked. @dependentKeyCompat is only necessary for native getters that are not decorated with 
@computed.

I suspect that this is being triggered from one of the third party addons we use as there are no cases where we use this decorator ourselves but the fact remains that patch updates of ember-source shouldn't introducing breaking changes and this one clearly does.

If you need more help reproducing this let me know and I can try and extract a little more info for you.

[rwjblue]

@fran-worley - Thanks for reporting! Sorry for the upgrade troubles here.

patch updates of ember-source shouldn't introducing breaking changes

I definitely agree that introducing a new assertion in a patch release is not great. I think from a usability perspective the only time it's "OK" to introduce new debug assertions like this is if it is telling you about something that is actually broken in your application code, that would have had downstream failures/errors already (but the error message would be misleading or useless).

---

Reading through the code (from #19138) I'm not quite sure if this falls into that same category though, as I think that if we remove the assertion things would still be operating correctly. Specifically:

ember.js/packages/@ember/object/compat.ts

Lines 12 to 37 in bde5e7a

	let wrapGetterSetter = function(target: object, key: string, desc: PropertyDescriptor) {
	let { get: originalGet } = desc;
	assert(
	'You attempted to use @dependentKeyCompat on a property that already has been decorated with either @computed or @tracked. @dependentKeyCompat is only necessary for native getters that are not decorated with @computed.',
	descriptorForProperty(target, key) === undefined
	);
	if (originalGet !== undefined) {
	desc.get = function() {
	let propertyTag = tagFor(this, key) as UpdatableTag;
	let ret;
	let tag = track(() => {
	ret = originalGet!.call(this);
	});
	updateTag(propertyTag, tag);
	consumeTag(tag);
	return ret;
	};
	}
	return desc;
	};

Is going to re-wrap the descriptors getter, which is definitely silly and a waste of time but not going to cause downstream failures / errors.

@pzuraq - Can you double check to confirm? If that is the case, then I'll remove the assertion from 3.20 / 3.22 series (leaving it in place for 3.23+) to ease the upgrade to 3.20.6+ for folks.

[pzuraq]

Yes, I think it should be fine to remove the assertion here, as you said it should just amount to doing a bit of extra work.

[rwjblue]

Submitted PR over in #19263.

[locks]

PR is merged but seems like Github didn't automatically close the issue. Thanks for the report @fran-worley and thanks for the quick turnaround from the team!

[fran-worley]

@locks sorry to bump, but this never actually got released... Given that v3.20 was LTS could we get a version bump? 🥺

[rwjblue]

Hmph, sorry I thought I had released it. Sorry about that! I'll try to get it done tomorrow morning. Reopening until we can confirm its released.

[Techn1x]

I think I hit this one today in my Ember 3.20.6 app. Updating to Ember 3.24.3 solved the problem.

Would be cool to see this backported to 3.20.7 if we can get that happening, would have saved me a bunch of time..

Friendly ping/reminder @rwjblue

[Techn1x]

Well, we're outside of the 3.20.x LTS bugfix window now :/

Guess it's not getting backported. I don't mean to be a grouch but it kind of devalues the benefit of being on an LTS version if we don't get these bugfixes.

Onto Ember 3.24 we go... :)

[rwjblue]

I'll get this backported to 3.20 today, I'm working on another couple of fixes that are critical for some apps I work on.

Guess it's not getting backported. I don't mean to be a grouch but it kind of devalues the benefit of being on an LTS version if we don't get these bugfixes.

You are 100% correct. I'm really sorry for the delays.

[rwjblue]

I've just tagged and kicked off publishing of https://github.com/emberjs/ember.js/releases/tag/v3.20.7, it should be published to npm once https://travis-ci.org/github/emberjs/ember.js/builds/773433834 CI job is completed.