Skip to content

Commit

Permalink
Fix owner passing
Browse files Browse the repository at this point in the history
  • Loading branch information
NullVoxPopuli committed Jun 23, 2023
1 parent cc6f8d1 commit e320cf8
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 3 deletions.
32 changes: 32 additions & 0 deletions .changeset/shy-badgers-melt.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
---
"ember-resources": patch
---

Fix situation where, when composing with blueprint/factory-creted Resources, the owner was not passed to the t`used`d resource.

<details><summay>Example from the added test</summary>

```js
const Now = resourceFactory((ms = 1000) =>
resource(({ on }) => {
let now = cell(nowDate);
let timer = setInterval(() => now.set(Date.now()), ms);

on.cleanup(() => clearInterval(timer));

return () => now.current;
})
);

const Stopwatch = resourceFactory((ms = 500) =>
resource(({ use }) => {
let time = use(Now(ms));

return () => format(time);
})
);

await render(<template><time>{{Stopwatch 250}}</time></template>);
```

</details>
17 changes: 17 additions & 0 deletions ember-resources/src/core/function-based/immediate-invocation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { createCache, getValue } from '@glimmer/tracking/primitives/cache';
import { associateDestroyableChild } from '@ember/destroyable';
// @ts-ignore
import { capabilities as helperCapabilities, invokeHelper, setHelperManager } from '@ember/helper';
import { dependencySatisfies, importSync, macroCondition } from '@embroider/macros';

import type { resource } from './resource';
import type { Cache } from './types';
Expand All @@ -18,6 +19,18 @@ interface State {
_?: any;
}

let setOwner: (context: unknown, owner: Owner) => void;

if (macroCondition(dependencySatisfies('ember-source', '>=4.12.0'))) {
// In no version of ember where `@ember/owner` tried to be imported did it exist
// if (macroCondition(false)) {
// Using 'any' here because importSync can't lookup types correctly
setOwner = (importSync('@ember/owner') as any).setOwner;
} else {
// Using 'any' here because importSync can't lookup types correctly
setOwner = (importSync('@ember/application') as any).setOwner;
}

class ResourceInvokerManager {
capabilities = helperCapabilities('3.23', {
hasValue: true,
Expand All @@ -36,9 +49,13 @@ class ResourceInvokerManager {
let cache = createCache(() => {
let resource = fn(...args.positional) as object;

setOwner(resource, this.owner);

return invokeHelper(cache, resource);
});

setOwner(cache, this.owner);

return { fn, args, cache, _: getValue(cache) };
}

Expand Down
16 changes: 15 additions & 1 deletion ember-resources/src/core/function-based/manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { associateDestroyableChild, destroy, registerDestructor } from '@ember/d
import { invokeHelper } from '@ember/helper';
// @ts-ignore
import { capabilities as helperCapabilities } from '@ember/helper';
import { dependencySatisfies, importSync, macroCondition } from '@embroider/macros';

import { INTERNAL } from './types';

Expand All @@ -18,6 +19,18 @@ import type {
} from './types';
import type Owner from '@ember/owner';

let getOwner: (context: unknown) => Owner | undefined;

if (macroCondition(dependencySatisfies('ember-source', '>=4.12.0'))) {
// In no version of ember where `@ember/owner` tried to be imported did it exist
// if (macroCondition(false)) {
// Using 'any' here because importSync can't lookup types correctly
getOwner = (importSync('@ember/owner') as any).getOwner;
} else {
// Using 'any' here because importSync can't lookup types correctly
getOwner = (importSync('@ember/application') as any).getOwner;
}

/**
* Note, a function-resource receives on object, hooks.
* We have to build that manually in this helper manager
Expand Down Expand Up @@ -46,6 +59,7 @@ class FunctionResourceManager {
let thisFn = fn.bind(null);
let previousFn: object;
let usableCache = new WeakMap<object, unknown>();
let owner = this.owner;

let cache = createCache(() => {
if (previousFn) {
Expand Down Expand Up @@ -77,7 +91,7 @@ class FunctionResourceManager {
destroy(previousCache);
}

let cache = invokeHelper(this.owner, usable);
let cache = invokeHelper(owner, usable);

associateDestroyableChild(currentFn, cache as object);

Expand Down
46 changes: 44 additions & 2 deletions test-app/tests/core/composition-test.gts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { tracked } from '@glimmer/tracking';
import { module, test } from 'qunit';
import { setupRenderingTest } from 'ember-qunit';

import { resource, cell } from 'ember-resources';
import { resource, cell, resourceFactory } from 'ember-resources';

// Will need to be a class for .current flattening / auto-rendering
interface Reactive<Value> {
Expand Down Expand Up @@ -53,6 +53,41 @@ module('Core | resource | use | rendering', function (hooks) {
assert.notEqual(first, second);
});

test('it works with the blueprint/factory', async function (assert) {
let nowDate = Date.now();
let format = (time: Reactive<number>) => formatter.format(time.current);

const Now = resourceFactory((ms = 1000) =>
resource(({ on }) => {
let now = cell(nowDate);
let timer = setInterval(() => now.set(Date.now()), ms);

on.cleanup(() => clearInterval(timer));

return () => now.current;
})
);

const Stopwatch = resourceFactory((ms = 500) =>
resource(({ use }) => {
let time = use(Now(ms));

return () => format(time);
})
);

await render(<template><time>{{Stopwatch 250}}</time></template>);

let first = formatter.format(Date.now());
assert.dom('time').hasText(first);

await wait(1010);

let second = formatter.format(Date.now());
assert.dom('time').hasText(second);
assert.notEqual(first, second);
});

test('lifecycle timing is appropriate', async function (assert) {
class State {
@tracked outerValue = 0;
Expand Down Expand Up @@ -111,7 +146,14 @@ module('Core | resource | use | rendering', function (hooks) {

await rerender();
assert.verifySteps(
['Inner:setup 1', 'Outer:setup 0', 'Outer:value:1', 'Inner:value:1', 'Outer:cleanup 0', 'Inner:cleanup 0'],
[
'Inner:setup 1',
'Outer:setup 0',
'Outer:value:1',
'Inner:value:1',
'Outer:cleanup 0',
'Inner:cleanup 0',
],
'outer is re-setup'
);

Expand Down

0 comments on commit e320cf8

Please sign in to comment.