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

disable maybeDeepFreeze #1909

Closed
kadosh1000 opened this issue Jul 16, 2017 · 22 comments
Closed

disable maybeDeepFreeze #1909

kadosh1000 opened this issue Jul 16, 2017 · 22 comments
Assignees

Comments

@kadosh1000
Copy link

Hi, I have an issue with apollo-client using angular 4.
I have a service fetching data from the server.

The thing is that in my component need to be able to add items to an array returned from the service.
But I keep getting errors indicating that the object and the array inside of it are frozen.

After deeply looking into the package I've seen that the object is being frozen in dev mode using the util maybeDeepFreeze. How can I override it? disable it? or bypass it?

@kadosh1000 kadosh1000 changed the title disable maybeDeepFreezt disable maybeDeepFreeze Jul 16, 2017
@intellix
Copy link
Contributor

intellix commented Jul 16, 2017

I think it's strange that this is only done on development rather than being consistent across environments. It'll lead to a few WTF moments I imagine.

I've also got a case where I'd like to disable deep-freezing for an individual query but I'm not sure how it'd affect the store.

Our use-case is that we're passing an object of properties to a third-party client who check that object and add/remove things. With our REST API it just worked but with Apollo it's frozen. I've basically got to manually destructure everything manually to ensure it can be edited like:

// Deeply unfreeze everything
const options = { ...data.options };
options.mobileParams = { ...options.mobileParams };
options.liveParams = { ...options.liveParams };

As is with third-parties, we can't tell them not to rely on the reference because they assume you haven't read the documentation/implemented it properly :D

@kadosh1000
Copy link
Author

@intellix I think maybe convert the individual queries I need to "unfreeze" to normal $http.post requests.
Why don't you use that?

@jbaxleyiii
Copy link
Contributor

@intellix if I'm not mistaken, the desire for it to only be present in development mode was for performance reasons when running it in production. That use case certainly shows problems with the current implementation though.

@stubailo do you have any thoughts here?

@kadosh1000
Copy link
Author

@jbaxleyiii @stubailo
I would really appreciate a quick reply on this issue, as it has really stuck my project.
Any workaround I can do for now? any suggestions?

I think the best option is to allow configure in the client configuration rather to enable the freeze feature or not, and let the default to be true. This way if there is no need to change the data retrieved than it's all remain the same.

@stale
Copy link

stale bot commented Aug 8, 2017

This issue has been automatically marked as stale becuase it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions to Apollo Client!

@mattapperson
Copy link

I am also more irritated with this then pleased it exists... :/

@stale
Copy link

stale bot commented Sep 3, 2017

This issue has been automatically marked as stale becuase it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions to Apollo Client!

@stale stale bot closed this as completed Sep 17, 2017
@stale
Copy link

stale bot commented Sep 17, 2017

This issue has been automatically closed because it has not had recent activity after being marked as stale. If you belive this issue is still a problem or should be reopened, please reopen it! Thank you for your contributions to Apollo Client!

@miguelcaravantes
Copy link

i want disable frezee

@Cito
Copy link

Cito commented Dec 3, 2017

See PrimeNG issue #4600 for a concrete case where this is problematic. You get an error message "Cannot add property _$visited, object is not extensible" when displaying the query result in a PrimeNG data table and there is no dataKey set. Would be nice if this would be configurable (globally and per query).

@OutThisLife
Copy link

👍

@intellix
Copy link
Contributor

intellix commented Feb 4, 2018

This is griefing me to no end. If you've got deeply nested edges that you need to merge during a subscribe you'll want to kill yourself. I'm thinking there has to be some sort of helper functionality exported to help. Consider how unreasonable it is to add a pet below:

const prev = {
  people: {
    edges: [
      {
        node: {
          id: 'UGVyc29uOjE=',
          name: 'Dominic',
          status: 'suicidal',
          pets: {
            edges: [
              {
                node: {
                  id: 'UGV0OjE=',
                  personId: 'UGVyc29uOjE=',
                  type: 'dog',
                  name: 'Boris',
                },
              },
            ],
          },
        },
      },
    ],
  },
};

deepFreeze(prev);

const newEdge = {
  node: {
    id: 'UGV0OjI=',
    personId: 'UGVyc29uOjE=',
    type: 'cat',
    name: 'Clyde',
  },
};

// As far as I know, you have to do all of the below:

const personEdge = prev.people.edges.find(e => e.node.id === newEdge.node.personId);
const newPersonEdge = {
  ...personEdge,
  node: {
    ...personEdge.node,
    pets: {
      ...personEdge.node.pets,
      edges: [
        ...personEdge.node.pets.edges,
        newEdge,
      ],
    },
  },
};

const newPersonEdges = prev.people.edges.filter(e => e.node.id !== newPersonEdge.node.id).concat(newPersonEdge);
let newPrev = {
  ...prev,
  people: {
    ...prev.people,
    edges: newPersonEdges,
  },
};

This reminds me of callback hell. I call it: spreading hell

@NathanWalker
Copy link

Curious of the same? Very frustrating here

@mwld
Copy link

mwld commented Mar 16, 2018

If you need to alter the returned frozen data you could clone it before subscribing. One easy way would be to use Lodash’s cloneDeep (https://lodash.com/docs#cloneDeep).

apollo.watchQuery(...)
    .valueChanges
    .map(({ data }) => _.cloneDeep(data))
        .subscribe(
            data => {
                // This data will not be frozen.
            });

@NathanWalker
Copy link

Thanks @mwld - preferably without an extra dependency and avoids spreading hell - an appropriate term @intellix

@julienvincent
Copy link

I think there should be a way to disable freezing. In my use case I am using immer within my mutation update functions, but am unable to due to this object freezing.

Can this issue be re-opened and properly discussed?

@hwillson
Copy link
Member

hwillson commented Aug 9, 2018

Hi all - re-opening. We're reconsidering the use of maybeDeepFreeze, and might remove it completely (making results mutable). Making sure state is immutable is great in a lot of cases, but the fact that we're only locking things down in dev/test environments, and not in prod, doesn't quite sit right. We're not deep freezing in production because it's expensive, but we also think it's important to align dev and prod environments (as we don't want to introduce hard to debug environment based issues). More details shortly - thanks!

@nagman
Copy link

nagman commented Aug 20, 2018

@hwillson Thanks for reconsidering it.
Any idea of when it will be implemented?
This is a really needed fix.

@michaeldoye
Copy link

This is a bit of a problem for one of my apps as well.

I am working around it like this for now:

return this.apollo.watchQuery({...})
  .valueChanges
  .pipe(map((data) => {
    const clonedObj = JSON.stringify(data);
    return JSON.parse(clonedObj);
  }));

which works fine but is not really ideal.

@smkhalsa
Copy link

@hwillson I am also dealing with the very frustrating consequences of this. Any update on making maybeDeepFreeze optional or removing it altogether?

@Christilut
Copy link

Same problem here. I don't see a reason to make properties read only, it just bothers me. Working around it like this:

      const items = new Array(...this.data.items)

      items.splice(i, 1, v)

      this.data.items = items

@hwillson hwillson added to do and removed to do labels Sep 4, 2018
hwillson added a commit that referenced this issue Sep 5, 2018
Apollo Client is currently deep freezing query results, making
query results immutable. This restriction was originally put
in place to reserve the possibility of potentially returning
direct cache references in query results. Since doing this
isn't currently planned, and there have been many requests to
make query results mutable, this commit disables query result
deep freezing.

Fixes #1909.
hwillson added a commit that referenced this issue Sep 5, 2018
* Stop deep freezing query results

Apollo Client is currently deep freezing query results, making
query results immutable. This restriction was originally put
in place to reserve the possibility of potentially returning
direct cache references in query results. Since doing this
isn't currently planned, and there have been many requests to
make query results mutable, this commit disables query result
deep freezing.

Fixes #1909.

* Changelog update
@hwillson
Copy link
Member

hwillson commented Sep 5, 2018

#3883 disables deep freezing; query results will be mutable in the next release. Thanks all!

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

No branches or pull requests