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

Allow consumer to further alter the behavior of isDraftable #405

Closed
thesnups opened this issue Aug 12, 2019 · 6 comments
Closed

Allow consumer to further alter the behavior of isDraftable #405

thesnups opened this issue Aug 12, 2019 · 6 comments

Comments

@thesnups
Copy link

🚀 Feature Proposal

I'm not 100% sure what this API would look like, but I'd like to see the mechanism for altering the behavior of isDraftable to be more configurable by the consumer. The current mechanism (applying the immerable symbol to objects that would otherwise be undraftable) is limiting for my use case.

Motivation

In my app there are two window contexts (the top window and an iframe window context) in which objects are created, and these objects frequently get passed between the two contexts. Immer's current isDraftable predicate only returns true for objects that are created in the same window context as itself, which means it only returns true for objects that were created in the top window. The immerable symbol could be used to solve this problem, but we would need to apply it in many places, whereas it would be simpler to have a way to tell Immer to take into account objects from either window context.

Example

Again, I'm not sure what it should look like, but one idea would be to allow the consumer to specify it's own isDraftable predicate that either A) overrides the built-in isDraftable, or B) is used as a "final" check when all of the existing isDraftable checks fail.

import { setIsDraftableOverride } from 'immer';

function myCustomIsDraftableFunction(val) {
  // ...
}

setIsDraftableOverride(myCustomIsDraftableFunction);

Let me know your thoughts!

@mweststrate
Copy link
Collaborator

I think that might be fixed if you would also share immer between the windows?

@thesnups
Copy link
Author

How do you propose doing this? I'm not seeing how that could work. The problem is on this line: https://github.com/immerjs/immer/blob/master/src/common.js#L31 Immer only checks for objects created with the object prototype in its current scope. Not objects created in other windows

@mweststrate
Copy link
Collaborator

mweststrate commented Sep 30, 2019 via email

@mweststrate
Copy link
Collaborator

For the object check: this check (from redux toolkit) might work cross frames?

function isPlainObject(value) {
  if (typeof value !== 'object' || value === null) return false;
  var proto = value;

  while (Object.getPrototypeOf(proto) !== null) {
    proto = Object.getPrototypeOf(proto);
  }

  return Object.getPrototypeOf(value) === proto;
}

@mweststrate
Copy link
Collaborator

Closing for inactivity

@atvoid
Copy link
Contributor

atvoid commented Mar 18, 2021

@mweststrate I think the one from lodash is better. what do you think ?

export function isPlainObject(value: any): boolean {
	if (!value || typeof value !== "object") return false
	const proto = Object.getPrototypeOf(value)
	if (proto === null) {
		return true
	}
	const Ctor =
		Object.hasOwnProperty.call(proto, "constructor") && proto.constructor
	return (
		typeof Ctor == "function" &&
		Function.toString.call(Ctor) === objectCtorString
	)
}

#767

mweststrate pushed a commit that referenced this issue Mar 19, 2021
…es. Fixes  #766 / #405

Co-authored-by: Yuehan Xu <yuex@microsoft.com>
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

3 participants