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

fix: LegacyHandler should provide unfrozen options to Adapters #8576

Merged
merged 5 commits into from
May 6, 2023

Conversation

kiosion
Copy link
Contributor

@kiosion kiosion commented Apr 18, 2023

Description

This PR addresses an issue with the LegacyHandler in 4.12 - The StoreRequestContext obj passed to legacy Adapters is frozen, causing issues when modifying request params, i.e., modifying the query param within an Adapter's query or urlForQuery methods. I've added a failing test case as a reproduction of how I ran into this issue.

Notes for the release

@kiosion kiosion force-pushed the legacy-handler-mutable-options branch from 561b688 to bcfe010 Compare April 19, 2023 15:48
@runspired runspired added 🎯 beta PR should be backported to beta 🎯 release PR should be backported to release 🎯 canary PR is targeting canary (default) 🏷️ bug This PR primarily fixes a reported issue lts-4-12 Long Term LTS Maintenance labels May 6, 2023
@runspired runspired merged commit d976bb0 into emberjs:main May 6, 2023
@runspired runspired changed the title LegacyHandler providing frozen options to Adapters fix: LegacyHandler should provide unfrozen options to Adapters May 6, 2023
runspired added a commit that referenced this pull request May 24, 2023
 #8576)


---------

Co-authored-by: Chris Thoburn <runspired@gmail.com>
@runspired runspired removed the lts-4-12 Long Term LTS Maintenance label May 24, 2023
@amk221
Copy link
Contributor

amk221 commented May 31, 2023

Thanks. I'm also experiencing this from:

urlForQuery(query) {
  let url = super.urlForQuery(...arguments);

  if (query.something) {
    url = `${url}/${query.something}`;
    delete query.something;
  }

  return url;
}
this.store.query('foo', {
  something: 'abc'
});

The actual URL used will be /foo/abc?something=abc

But it should be /foo/abc

runspired added a commit that referenced this pull request Jun 10, 2023
 #8576)


---------

Co-authored-by: Chris Thoburn <runspired@gmail.com>
runspired added a commit that referenced this pull request Jun 10, 2023
* fix: restore Store extends EmberObject :( (#8594)

* fix: restore Store extends EmberObject :(

* fix EmberObject bs

* fix: LegacyHandler should provide unfrozen options to Adapters (resolves #8576)


---------

Co-authored-by: Chris Thoburn <runspired@gmail.com>

* fix: dont share promise cache for all fields (#8597)

* fix: docs generation should maintain a stable relative path (#8598)

* docs: fix forgotten references to FetchManager (#8601)

* Avoid unnecessary identity notification when record is saved (#8566)

* Add failing test

* Ignore JetBrains IDE files

* Revise failing test

* Call setRecordId only for new records

* Add (failing) test for updated id from server

* update test

* error when appropriate, thread context to give cache responsibility

* fix lint

---------

Co-authored-by: Chris Thoburn <runspired@gmail.com>

* fix backport of identity notification fix

* There are cases where payload is an object, and normalizeErrorRespons… (#8621)

* There are cases where payload is an object, and normalizeErrorResponse ends up returning [object Object] for "detail"

* Add comment about JSON.stringify

* Update rest.ts

---------

Co-authored-by: Maxim <hi@kio.dev>
Co-authored-by: Robby Morgan <robby.morgan@yahoo.com>
Co-authored-by: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎯 beta PR should be backported to beta 🎯 canary PR is targeting canary (default) 🎯 release PR should be backported to release 🏷️ bug This PR primarily fixes a reported issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants