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

Read field policy ignored #9293

Open
CarsonF opened this issue Jan 11, 2022 · 10 comments
Open

Read field policy ignored #9293

CarsonF opened this issue Jan 11, 2022 · 10 comments

Comments

@CarsonF
Copy link
Contributor

CarsonF commented Jan 11, 2022

Setup:

  1. I have a query like:
    query EngagementDetail($id: ID!) {
      engagement(id: $id) {
        id
        createdAt
        products {
          total
          items { # ProductList[]
            id
            label
  2. I have a field policy on Engagement.createdAt to convert it from an ISO string to a Date object.

Problem:

  1. I have a mutation that creates a product.
  2. On that mutation is an update function that adds the new product to products.items array and increments the total count.
  3. The mutation mistakenly only returns the product id, but not the label property.
  4. When returning to the Engagement detail page, the query determines it doesn't have all the data and makes a network request.
  5. However, the read function on the field policy is ignored once the server data is retrieved.
  6. An error is thrown during react render because I'm expecting createdAt to be a Date object but it is a string.

Related reproduction/workaround notes

  • "Fixing" the mutation to include the label property, causes the query return data from cache with the read policy applied.
  • Also with out the cache.modify() call on the product create mutation updater function this doesn't happen - because the cached data for the query is still complete I'm assuming.

Intended outcome:

read function on field policy should be used after network request finishes to resolve the complete data requested in query.

Actual outcome:

read function on field policy is never invoked.

How to reproduce the issue:

Versions

  System:
    OS: macOS 11.5.2
  Binaries:
    Node: 16.13.0 - /private/var/folders/d6/mv12j6h94531qtzx4tt9h1vc0000gp/T/xfs-b8bb2665/node
    Yarn: 3.1.1 - /private/var/folders/d6/mv12j6h94531qtzx4tt9h1vc0000gp/T/xfs-b8bb2665/yarn
    npm: 8.1.0 - /usr/local/Cellar/node@16/16.13.0/bin/npm
  Browsers:
    Chrome: 97.0.4692.71
    Firefox: 79.0
    Safari: 14.1.2
  npmPackages:
    @apollo/client: ^3.5.6 => 3.5.6 
@CarsonF
Copy link
Contributor Author

CarsonF commented Jan 11, 2022

Ok I just found out the new item is not returned from the engagement query, due to the item not being in the first page of the products list.

Here are 3 frames related to this query after the mutation is executed.
Screen Shot 2022-01-11 at 3 13 39 PM
Screen Shot 2022-01-11 at 3 14 23 PM
Screen Shot 2022-01-11 at 3 14 56 PM

We can see that the last one still has the field error. It's not logged however, because there is other data.

It is still surprising that this affects a completely unrelated field createdAt. If Apollo thinks it's ok to move on silently in this case it should apply the field policies of all/unrelated fields.

I understand that this is a weird use case because I'm doing something wrong. It just took me a while to find the problem; I was only debugging the symptom (read policy), since Apollo didn't warning me about the missing field.

@bignimbus
Copy link
Contributor

Hi @CarsonF 👋🏻 thanks for your patience! Do you still need support here? If so, I'm not 100% certain what we might be able to do with the info given - can you share a runnable reproduction so we can see the issue on our end? Thanks so much!

@CarsonF
Copy link
Contributor Author

CarsonF commented Dec 12, 2022

Ha it's been a bit; I'll have to refresh myself. I'll see if I can come up with a repro this week.

@CarsonF
Copy link
Contributor Author

CarsonF commented Feb 18, 2023

Hey @bignimbus I was able to distill a reproduction here:

https://github.com/CarsonF/apollo-reproduction-read-policy-ignored

If you just add a single person in the example you'll see that the field policy to convert the string to a Date object is skipped for the list.

@alamchrstn
Copy link

hi what's the latest status on this issue?

@jerelmiller
Copy link
Member

Hey all 👋

We just released v3.9.0 which contains a fix for this issue (#11202). As such, I'm going to go ahead and close this out. Thanks!

Copy link
Contributor

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Client usage and allow us to serve you better.

@alessbell
Copy link
Contributor

Hi all 👋 Unfortunately, the fix in #11202 broke some behavior as reported in #11560 for several users. We are reverting the fix for this issue via #11576 and will be revisiting this issue soon. Thanks for your patience.

@alessbell alessbell reopened this Feb 6, 2024
@mikevoets
Copy link

Hello @alessbell just checking to see if you know - is there any update on this possibly?

@alessbell
Copy link
Contributor

Hi @mikevoets! No update at this time - we're in the midst of conference season and tackling some other priorities like data masking, but I've added this to our next meeting agenda so we can revisit it. Thanks for the nudge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants