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

Ember-resources example with composition of remoteData with async authorization headers? #1187

Open
johanrd opened this issue Aug 3, 2023 · 7 comments

Comments

@johanrd
Copy link
Contributor

johanrd commented Aug 3, 2023

Hi. Thanks for the fun and useful https://tutorial.glimdown.com.

When looking at the remote-data and keeping latest examples I get exited about the simplicity and composability.

However, I struggled to wrap my head around good practices with async inputs for the remoteData function. In my case I use firebase getIdToken , which returns a promise.

Ideally I'd like to compose a RemoteData function that awaits the results of getIdToken().

// expecting this to return with `isLoading` state if getIdToken is pending or if remoteData is pending:
@use myData = resource(async (hooks) => {
  let token = await getAuth().currentUser?.getIdToken()
  return remoteData<string>(hooks, this.args.fetchUrl, {
    headers: {
      Authorization: `Bearer ${token}`
      }
    }
  )
})

@use latest = keepLatest({
  value: () => this.myData.value,
  when: () => this.myData.isLoading,
});

However, simply marking the function and awaiting the token, makes this.myData return as Promise<State<string>>, not as State<string>, so I'm probably on the wrong tracks here.

Any ideas for better composibilbity? Wrapping it in a resourceFactory? Or do I anyways need to create a custom wrapper util that returns a remoteData like State if getIdToken is running?

Posting here, since I think it could be a useful exercise in the glimdown tutorial. Thanks!

@johanrd
Copy link
Contributor Author

johanrd commented Aug 3, 2023

ok, I guess I could use trackedFunction, but then I'd have to implement the AbortController myself.

request = trackedFunction(this, async () => {
  const token = await getAuth().currentUser?.getIdToken()
  const headers = new Headers({"Authorization": `Bearer ${token}`})
  let response = await fetch(this.args.fetchUrl, {headers});
  let data = await response.json();
  return data;
});

@use latest = keepLatest({
  value: () => this.request.value,
  when: () => this.request.isLoading,
});

An example with a combination of trackedFunction and remoteData could be powerful, I guess.

@NullVoxPopuli
Copy link
Owner

NullVoxPopuli commented Aug 3, 2023

I love this suggestion!

I think what I want to provide a way to do is something that ultimately looks like this, keeping in mind that classes are not the primary way to use resources, but only one of a few.

const Request = resourceFactory((urlOrUrlFn: string | () => string) => {
  return resource(({ use }) => {
    let tokenRequest = use(TrackedFunction(async () => getAuth()?.currentUser?.getIdToken()));

    if (tokenRequest.isLoading) {
      return tokenRequest; // a State<string>
    }
  
    let headers = new Headers({ Authorization: `Bearer ${tokenRequest.value}` });
    
    let actualRequest = use(RemoteData(urlOrUrlFn, { headers }));
    
    return actualRequest; // State<unknown>, I think
    // the states are different, but I think they both have isLoading and 'value'
    // these should probably be the same State, so you can be certain of all the 
    // promise-handling properties being the same so consumer-facing usage
    // is cleaner
  });
});

export class Demo {
  @use request = Request(() => this.args.fetchUrl));
  
  @use latest = keepLatest({
    value: () => this.request.value,
    when: () => this.request.isLoading,
  });
}

This is some psuedo api as TrackedFunction doesn't exist in a way that is compatible with use yet -- but soon!

@NullVoxPopuli
Copy link
Owner

NullVoxPopuli commented Aug 13, 2023

This has been implemented in ember-resources -- idk if you want to write up tutorial text? could be helpful for others!
https://github.com/NullVoxPopuli/ember-resources/blob/main/ember-resources/CHANGELOG.md#640

@johanrd
Copy link
Contributor Author

johanrd commented Aug 15, 2023

ok, great, thanks. I will give it a go.

I ended up having a little issue with latest being returned twice, though:

  1. First when the resource is first needed (e.g. from a get())
  2. Then after the actualRequest is returned.

I have some expensive downstream calculations and rendering that takes effect when latest is updated. Is there any good practice to make sure that latest downstream calculations are only triggered on 2), not 1)?

I can think of two solutions:

  1. An ember-concurrency task where both the fetching and the expensive calculations are in a nested task, then updating a @tracked property only at the end to trigger a re-render. (This is in practice an imperative pattern, not reactive, i guess)
  2. Investigate the Formula concept in starbeam.js. Do you have any experience how ready that is? I see there are some mentions of it in the ember-resource docs, but not sure how intentional that is.

Thanks,

@NullVoxPopuli
Copy link
Owner

NullVoxPopuli commented Aug 15, 2023

with latest being returned twice, though:

this is as optimized as it can be 🎉
(and found out the two values 💪 )

Is there any good practice to make sure that latest downstream calculations are only triggered on 2), not 1)?

The path here is to handle the non-value case, which you already had to do, but was hidden! (and often forgotten!)

@cached
get expensiveCalculation() {
  if (!this.latest) {
    // or whatever the initial value
    return null; // or {} or [] or whatever is appropriate
  }

  return doSomethingExpensive();
}

This is in practice an imperative pattern, not reactive, i guess

Correct, additionally, ember-concurrency setting properties introduces "hidden states" -- you still have a period of time where your values don't have what you want (until the appropriate thing is awaited), but it's the same situation, just wrapped differently, with implicit states.

Investigate the Formula concept in starbeam.js. Do you have any experience how ready that is?

yeah, it's kind of the same as @cached getters. so @cached is likely what you want

@johanrd
Copy link
Contributor Author

johanrd commented Aug 15, 2023

@NullVoxPopuli thanks for your reply.

Yes, I guess checking for !this.latest helps to avoid heavy double renders/calculations upon the first fetch, however caching the getter does not help, since subsequent fetchUrl invalidations rerun the getter every time this.latest is invalidated by fetchUrl. (I have some queryParams are that have {refreshModel: true} (for instance dates), and all queryParams seem to be invalidated when the model refreshes.)

To avoid subsequent double calculations/renders, I feel a need to do track the if @fetchUrl (e.g. tracked from dates in query params) is unequal to the last used @fetchUrl, and only then invalidate data imperatively:

RemoteData component:

@tracked lastFetchedUrl?: string;
@tracked lastData? : any[]

fetchRemoteData = task(async () => {
  const url = this.args.fetchUrl
  if (url === this.lastFetchedUrl) return
  const response = await fetch(url)
  const json = await response.json()
  this.lastFetchedUrl = url
  this.lastData = json 
})

<template>
  {{did-insert (perform this.fetchRemoteData) }}
  {{did-update (perform this.fetchRemoteData) @fetchUrl}}
  {{yield this.lastData this.lastFetchedUrl}}
</template>
}
<RemoteData
  @fetchUrl={{this.fetchUrl}}
  as |data lastFetchedUrl| 
>
  <ExpensiveCalculation
    @data={{data}}
    as |calculatedData|
  />
    <ExpensiveRender
      @data={{calculatedData}}
    />
  <ExpensiveCalculation>
</RemoteData>

This works, but it feels wrong to work against the grain of reactive patterns. But perhaps it has to be done imperatively for full control. ?

@NullVoxPopuli
Copy link
Owner

NullVoxPopuli commented Aug 16, 2023

the pattern you describe is specific, and makes sense that you're running in to a bit of a rough edge with some primitives.

"Keeping latest" is what I've called what you're doing: tutorial page | repl

Because you want to only update a value when you have a new value to update with, and ignore all the intermediate stuff.

So, to adapt your example code:

class Demo {
  @use currentRequest = RemoteData(() => this.args.fetchUrl));
  @use data = keepLatest({ 
    value: () => this.request.value,
    when: () => this.request.isLoading,
  });
  
  <template>
    {{yield this.data}}
  </template>
}

I like this pattern because it allows you to still indicate loading or error state while keeping your data displayed

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