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

Query arity check fails, leading to recordArray/options being dropped #6232

Closed
loganrosen opened this issue Jul 9, 2019 · 3 comments
Closed
Assignees
Labels
🏷️ bug This PR primarily fixes a reported issue

Comments

@loganrosen
Copy link
Contributor

loganrosen commented Jul 9, 2019

Reproduction

@runspired is aware of the specific issue here and said that reproduction steps are unnecessary for this issue.

Description

In the underlying _query() function used when calling store.query(), it makes an arity check for the number of arguments of query() in the adapter to see if it should pass along recordArray and options as fourth and fifth parameters. However, this length check does not work unless native classes are used. In a local test case, even when the adapter has query(store, type, query, recordArray, options), this check sees it as a superWrapper with a length of 0. This causes it to only pass along the store, type, and query arguments, and it doesn't create/pass a record array, nor does it pass options.

According to @runspired, this appears to have been broken since 3.2.

Versions

@linkedin/ember-pem@0.0.0 /Users/lrosen/ember-pem_trunk
└── ember-source@3.10.1 

@linkedin/ember-pem@0.0.0 /Users/lrosen/ember-pem_trunk
└── ember-cli@3.10.1 

@linkedin/ember-pem@0.0.0 /Users/lrosen/ember-pem_trunk
└── ember-data@3.10.0 
@runspired runspired self-assigned this Jul 9, 2019
@runspired
Copy link
Contributor

runspired commented Jul 9, 2019

The resolution here is to drop the arity check entirely as there is no longer a way to do this when things extend from EmberObject.

TL;DR wrappedFunction is no longer public and is hidden inside a closure, so any function calling this._super which gets wrapped will fail arity checks, regardless of whether there is a super to call or not.

@runspired
Copy link
Contributor

@malatin3 This is the answer for your question in #6248, if you have a cycle to spare to implement the fix proposed here please do :)

@loganrosen
Copy link
Contributor Author

loganrosen commented Jul 15, 2019

@runspired @malatin3 I took a stab at this in #6250.

runspired pushed a commit that referenced this issue Jul 15, 2019
This addresses the issue described in #6232. The arity check that decides whether to create a record array and pass options to adapter.query fails when the adapter extends EmberObject.
pete-the-pete pushed a commit to pete-the-pete/data that referenced this issue Jul 23, 2019
This addresses the issue described in emberjs#6232. The arity check that decides whether to create a record array and pass options to adapter.query fails when the adapter extends EmberObject.
@runspired runspired added 🏷️ bug This PR primarily fixes a reported issue and removed Bug labels Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bug This PR primarily fixes a reported issue
Projects
None yet
Development

No branches or pull requests

2 participants