[FEAT adapterOptions] Ensure adapterOptions use is possible throughout #5627
Conversation
|
An external demo of this appearing to work can be found here: |
This comment has been minimized.
This comment has been minimized.
|
The two test-related commits are done with different styles.
Let me know which pattern you prefer, I can use it for both. |
The entire original |
| @@ -231,17 +231,17 @@ export default class BelongsToRelationship extends Relationship { | |||
| Other calls must conform to the typical expectations, for instance, sync relationships | |||
| expect that their data is already loaded. | |||
| */ | |||
| getData(isForcedReload = false) { | |||
| getData(options = {}) { | |||
There was a problem hiding this comment.
let's leave isForcedReload as the second argument here, as it should not be leaked to end users.
There was a problem hiding this comment.
options does not need a defaultValue
| @@ -270,7 +270,7 @@ export default class BelongsToRelationship extends Relationship { | |||
| "' with id " + | |||
| this.internalModel.id + | |||
| ' but some of the associated records were not loaded. Either make sure they are all loaded together with the parent record, or specify that the relationship is async (`DS.belongsTo({ async: true })`)', | |||
| record === null || !record.get('isEmpty') || isForcedReload | |||
| record === null || !record.get('isEmpty') || options.reload === true | |||
There was a problem hiding this comment.
let's keep isForcedReload
| @@ -344,17 +344,17 @@ export default class ManyRelationship extends Relationship { | |||
| }); | |||
| } | |||
|
|
|||
| getData(isForcedReload = false) { | |||
| getData(options = {}) { | |||
There was a problem hiding this comment.
same as for belongsTo, make isForcedReload the second arg.
There was a problem hiding this comment.
also, options does not need to have a default value
| @@ -379,7 +379,7 @@ export default class ManyRelationship extends Relationship { | |||
| }' with id ${ | |||
| this.internalModel.id | |||
| } but some of the associated records were not loaded. Either make sure they are all loaded together with the parent record, or specify that the relationship is async (\`DS.hasMany({ async: true })\`)`, | |||
| this.allInverseRecordsAreLoaded || isForcedReload | |||
| this.allInverseRecordsAreLoaded || options.reload === true | |||
There was a problem hiding this comment.
same as belongsTo, let's keep isForcedReload here
| @@ -554,7 +554,8 @@ export default class Relationship { | |||
|
|
|||
| this.setHasFailedLoadAttempt(false); | |||
| this.setShouldForceReload(true); | |||
| this.getData(true); | |||
| options.reload = true; | |||
| this.getData(options); | |||
There was a problem hiding this comment.
let's make this this.getData(options, true). We don't want to leak the ability to add reload as an option to calls to load, and the reason for this flag is to enforce that only this specific call to getData is considered a reload.
| reload() { | ||
| return this.hasManyRelationship.reload(); | ||
| reload(options = {}) { | ||
| options.reload = true; |
There was a problem hiding this comment.
we can delete the default value assignment and the options.reload = true assignment
| @@ -545,7 +545,7 @@ export default class Relationship { | |||
| this.setRelationshipIsStale(true); | |||
| } | |||
|
|
|||
| reload() { | |||
| reload(options = {}) { | |||
There was a problem hiding this comment.
options does not need a default value here
|
The only remaining things are related to |
| @@ -395,11 +417,22 @@ export default class BelongsToReference extends Reference { | |||
| }); | |||
| ``` | |||
|
|
|||
| You may also pass in an options object whose properties will be | |||
| fed forward. This enables you to pass `adapterOptions` into a | |||
| a reference. A full example can be found in the `load` method. | |||
There was a problem hiding this comment.
" into the request given to the adapter via the reference" might be more clear in these docs.
runspired
changed the title
Addressing this comment: #3700 (comment)