Feature proposal / user-defined request parameters / pull request

Jens-dojo commented 9 years ago

Hello,

this is the first time that I work with Git and that I try to contribute to such a project, so please be patient with me :-)

Request / REST Store.

It would be useful if it were possible to define additional ajax reqest parameters that are sent with every ajax request.

Currently, one can define additional headers, but not URL parameters.

I have made changes to the source code.

The dstore.Request gets a new

parameters

property.

// parameters: Object // A set of key/value pairs. // Additional user-defined parameters to pass in all requests to the server. These can be overridden // by passing additional parameters to calls to the store. // // Example: // // parameters:{ // myPara1:'value1', // myPara2:'value2' // }

The code changes are rather simpel and they work fine for me. I would be happy if you could include that new feature.

Jens

neonstalwart commented 9 years ago

@Jens-dojo is overriding/extending the _renderUrl method a problem for you?

var params = { param1: 'value', param2: 'another' };
var Store = Rest.createSubclass({
  _renderUrl: function () {
    var url = this.inherited(arguments),
      joinChar = url.indexOf('?') >= 0 ? '&' : '?';

    url += joinChar + ioQuery.objectToQuery(params);
    return url;
  }
});

Jens-dojo commented 9 years ago

Hello,

now that is much more elegant, with "ioQuery". I didn't know that function. Where is it?

I still feel that it might be useful to have "Parameters" it as a ready-to-go feature of the Request?!

Especially as overwriting a private function should be avoided by the end-user.

And it doesn't create any kind of useless overhead...

I could change my code based on yours and make a new pull request ?

neonstalwart commented 9 years ago

ioQuery is the exports of the dojo/io-query module.

as for overriding _renderUrl, this is the way it's intended to be used. trying to account for every possible case people may have will end up bloating the library code. instead, users should extend the library classes to create their own classes which meet their specific needs.

it's not up to me whether or not your PR will be accepted but in general, if something is already possible by some other means then unless a feature is really compelling it's unlikely to be merged. your needs can be met by a subclass so we'll see if someone else thinks your case is compelling enough to merge it.

Jens-dojo commented 9 years ago

Dear Ben,

I agree to all what you said.

If a user can find a solution to a problem by subclassing and overriding public functions - using the documentation, that's all fine for me.

But we are talking about private functions that are not (yet / never?) mentioned in the APIs / documents.

So there is no chance for the end-user to find a solution to this without digging (deeply) into the source code.

Jens

kriszyp commented 9 years ago

It seems like we should remove the prefixing for all these _render* methods, and make them public/documented.

On Thu, Dec 18, 2014 at 8:18 AM, Jens-dojo notifications@github.com wrote:

Dear Ben,

I agree to all what you said.

If a user can find a solution to a problem by subclassing and overriding public functions - using the documentation, that's all fine for me.

But we are talking about private functions that are not (yet / never?) mentioned in the APIs / documents.

So there is no chance for the end-user to find a solution to this without digging (deeply) into the source code.

Jens

— Reply to this email directly or view it on GitHub https://github.com/SitePen/dstore/issues/79#issuecomment-67500062.

Jens-dojo commented 9 years ago

That might be one possibility. But still: a class that is based on http requests. Shouldn't it be a built-in feature to easily set additional parameters? That seems not so exotic....

The "headers" option exists.

And when - next step - we implement the feature request for "choose between GET and POST", these methods will be handy, as you can set parameters without worrying about GET/POST.

EToreo commented 9 years ago

I agree with the idea of removing the underscore from the _render* functions. When I see an underscore it signals to me that I should look elsewhere for a solution.

brandonpayton commented 9 years ago

Unfortunately, due to an oversight, _renderUrl is not currently called in Request.

This issue and #68 started me thinking about a more general solution for these sorts of feature requests. I pushed an http-request-customization branch that removes the underscore prefixes from those methods, clarifies their names, and adds a number of new hooks. After the change, the following hooks are available in Request and Rest:

My hope is that these hooks will give sufficient flexibility for less common cases like #68 and cases like this where you want to add to the query params. What do you think of these?

Adding support for additional query params could be done with a small mixin:

var FetchParams = declare(null, {
    fetchParams: null,
    renderFetchQueryParams: function () {
        var queryParams = this.inherited(arguments);
        this.fetchParams && queryParams.push.apply(queryParams, this.fetchparams);
        return queryParams;
    }
});

Personally, I am not sure it is worth adding a dstore feature to support additional query params, but perhaps I am wrong and there are more people out there with similar needs. For now, I believe this is a feature that should be implemented locally in your project, as a mixin or by passing overriding methods during instantiation. What do others think about this?

brandonpayton commented 9 years ago

One note: I don't love the name renderFetchQueryParams, but it seemed too generic to name a fetch-specific thing renderQueryParams.

neonstalwart commented 9 years ago

my preference would be to encourage the use of existing abstractions rather than make new ones. so in thinking of what's needed here, the question to ask might be "is there a way this can already be done without making a new abstraction in dstore?" and if so, then the consideration becomes how convenient the alternative is and then only add abstractions that add significant convenience or which make something possible that was not otherwise possible.

@brandonpayton i'm thinking your suggested API is now overly abstract. especially since much of it seems to repeat abstraction you can already get from a requestProvider - e.g. issue*Request can all be handled with a request provider since the request provider can intercept the requests and arbitrarily change them as needed. as an example, if you needed to POST all your store.get requests then the provider could do that for you or if you needed to add authentication headers or url params, a request provider can already do that. the calls to request.* are already using an abstraction that is then concretely fulfilled by a request provider. so rather than change the call to request.* to suit the xhr provider, implement your own provider that translates the requests as needed.

i think if you start to think about whether all this abstraction belongs on a store, you may end up thinking "we need something to encapsulate the issue*Request methods since they don't really belong on the store" and then you arrive at what is already in request providers :smile: - we already have an amount of flexibility by using dojo/request.

Personally, I am not sure it is worth adding a dstore feature to support additional query params,

i agree to not add so many things to dstore but simply make things possible so that consumers can use things like subclassing or request providers to get what they may need.

neonstalwart commented 9 years ago

@brandonpayton one more thing... it's been advised quite a few times to override _ methods in dstore to get what's needed. if those methods go away, would that be a breaking change to the API which necessitates a major version bump?

kfranqueiro commented 9 years ago

Huge +1 to "use existing abstractions rather than create new ones". The fact that dstore uses dojo/request rather than dojo/_base/xhr (or any other direct implementation, legacy or otherwise) is a huge advantage it has over dojo/store, and we should be embracing that, unless there are legitimate reasons that a request provider would not be able to solve these problems, or concerns that it would complicate issues elsewhere.

I imagine #68 might be solvable via custom providers as well (intercept the URL in question and change options.method). (Edit: oh look, Ben already said that there like 2 weeks ago, I can't read during holidays. Sorry Ben!)

Also, RE underscore-prefixed methods, I thought it was generally accepted that these are often not so much "private" as they are "internal" and are open to being extended by subclasses. There's just a possibility that the behavior may change in the future.

Jens-dojo commented 9 years ago

Hello,

thank you for all the very interesting suggestions! I learned a lot.

Maybe we should close this issue as well as #68 ? And have a hint in the Request-Store documents that everything request-related should be changed in a "request provider". (an example would be nice...) It might even be a good idea to remove the "header options" in the Request Store - to stay consistent with the above approach?!

Merry Christmas to all of you! Jens

brandonpayton commented 9 years ago

@neonstalwart and @kfranqueiro, thanks for the feedback. Here's a wall of text for the holidays. :)

We seem to be on the same page about not introducing needless abstractions, but I'm not sure I agree with regard to the Request/Rest hooks. After thinking through these issues and sketching out my comments, I am closer to agreeing than I was, but I'm still not there. Here is where I'm coming from.

The purpose of the hooks is to present existing Request/Rest behavior in a way that can be easily overridden. The methods add no new behavior, are simple, and represent the truth of what an HTTP collection needs to do for each operation. If dojo/request providers were a good solution, I would agree that it is wasteful to expose these methods, but I believe request providers are a weak solution.

My biggest concern is that dojo/request providers operate at the level of an HTTP abstraction, not at a collection level. This has the following consequences:

dojo/request providers only have access to the What of a request, but when customizing the request for a collection operation, you want access to the Why. The collection hooks have this naturally via each operation's primary argument and its options object.
- In some cases, you can infer the Why from the What, but code that makes inferences is by nature less readable and more error prone than code that has direct access to the information.
- Example: dstore/Rest issues a POST for both a put with options.incremental and a put for an object with no ID.
- To discern what operation the request is for, a request provider has to match on the HTTP method, parse the request body, examine the resulting object, have the knowledge of what field(s) constitute an ID in the collection, and use the presence of an ID as an indicator of a new object or a partial update of an existing object.
- In contrast, a developer overriding issuePutRequest has natural, direct access to the object, this.getIdentity, and options.incremental. It will be familiar, collection-oriented code.
Using a dojo/request provider requires mapping each HTTP request to the correct collection operation based on the HTTP method, headers, and parsed request body. This is additional code that has to be written, and doing it correctly requires in-depth knowledge of what HTTP requests the collection issues by default. Providing collection hooks alleviates this burden, avoids coupling to the default HTTP request behavior, and leaves the actual customization code neatly organized in operation-specific methods.

Some other concerns are:

Since dojo/request providers are registered globally, each instance of a collection using dojo/request interacts with global state. This likely won't be an issue because even if two collections are created with the same target, they will likely introduce the same request provider implementation and continue to work properly regardless of which registered the winning request provider. However, I believe this just means we're lucky, not that we created a well-encapsulated solution. Overriding collection methods naturally provides per-instance encapsulation and does not rely on modifying global state.
dojo/request providers need to be removed from the registry when your collection is no longer used, but dstore does not offer anything like a destroy method (and it could be difficult or impossible to know when all subcollections have fallen out of use). Unless the collection developer is conscious of this and provides their own solution, request providers will leak.

What are your thoughts?

Jens-dojo commented 9 years ago

Hello,

I don't have the inside knowledge into dstore that you have - but as the library should also be used by "ordinary" developers, here is my feedback.

1) I like the idea of doing everything in a separate request provider

2) But after trying that; I don't see how I can use ONE specific request provider for ONE specific store. At least not if the target URL is not unique !

In my web app, there is for example ONE java servlet on the remote host that handles different data requests under ONE url and with different url post/get parameters.

Idea: Wouldn't it be a good compromise between Ben's and Brandon's idea to have a

requestProvider

property in the Request store object ?

If not set, dstore works unchanged.

if set, dstore uses the provided requestProvider Object.

So users could easily subclass RequestStore and add their own request provider in that subclass. And with that new property, no need to override the existing _request code. And no need to work with globally registered request providers.

BTW: ExtJs where I come from does it that way: the stores have a "proxy" property for "proxy objects" that are similar to requests in dojo.

Something like

function (request, lang, arrayUtil, JSON, declare, Store, QueryResults) {

    var push = [].push;

 NEW:      var standardRequest=request;

    return declare(Store, {

.......
_request: function (kwArgs) {
            kwArgs = kwArgs || {};
NEW:
var request=standardRequest;
if (this.requestProvider)
  request=this.requestProvider;
......

That small change would be the solution for all of my problems, even for #68. There could even be a collection of RequestProviders one day for different scenarios that people could just plug-in into the new RequestStore config object ! (e.g. POST provider, communication with different web services etc).

What do you think?

Jens

neonstalwart commented 9 years ago

@brandonpayton i wanted to respond directly to some of your comments but my responses are kind of terse and scattered which may make them seem hostile - don't take them that way, i'm just extremely busy right now. so first an attempt at a general response and then some specifics.

URLs are an external dependency which are often not controlled by the person writing code that uses a dstore collection but the behavior of those URLs should be well-defined and consistent
in a "good" architecture, one collection is going to be the only thing interacting with a particular URL. i.e. you wouldn't get, update, or delete resources from a target without using the collection associated with those resources
you likely won't have 2 collections using the same target (although #81 is a really interesting case but _renderUrl would be enough to take care of that one) so you don't need instance-specific differences for 2 collections that have the same target.
dstore/Rest makes unambiguous requests using dojo/request that completely convey the intent of the method called on the collection. these requests can easily be understood by a request provider without needing to have intimate knowledge of the resources. i would expect very few cases where it was not possible for a request provider to be able to adapt the request from a collection into a suitable HTTP request to an external data source. a brief overview for those not familiar with the protocol:

verb	url	meaning
GET	`<target>`	get a collection of resources located at target, headers may be used to indicate pagination and the querystring used to indicate filtering
GET	`<target> + <id>`	get a specific resource with the id `<id>`
PUT	`<target> + <id>`	update a specific resource, the body of the request contains the full representation of that resource
POST	`<target> + <id>`	update a specific resource, the body of the request contains a partial update of the resource
POST	`<target>`	add a new resource, the body of the request contains the full representation of that resource
DELETE	`<target> + <id>`	delete the resource with the id `<id>`

some of these semantics can be changed by customizations to the collection but, given the bullet points above, it would not be a bad practice for the request provider and the collection to be working together rather than in complete isolation. as @Jens-dojo suggests it may be possible to build up a library of these request providers for well-known data sources and people could use a dstore/Rest instance combined with one of these request providers and be up and running without any further customization.

another concern i have with such fine-grained hooks is that it's veering into 2nd system syndrome. dojo/store had a very concise API but since dojo/request didn't exist at the time when the stores were written, they did not use dojo/request. introducing dstore/Request and using dojo/request should be sufficient to address the vast majority of issues that needed to be worked around with dojo/store and HTTP data sources.

The methods ... represent the truth of what an HTTP collection needs to do for each operation.

the requests currently being made unambiguously represent the semantics of each operation. it's simple for a request provider to decode what is being asked for.

My biggest concern is that dojo/request providers operate at the level of an HTTP abstraction, not at a collection level.

this is actually exactly why they are the right place to do this - HTTP abstraction should be happening in the place that is responsible for doing HTTP requests. if i was only allowed one point in my argument, this would be it.

Example: dstore/Rest issues a POST for both a put with options.incremental and a put for an object with no ID. To discern what operation the request is for, a request provider has to match on the HTTP method, parse the request body, examine the resulting object, have the knowledge of what field(s) constitute an ID in the collection, and use the presence of an ID as an indicator of a new object or a partial update of an existing object.

not quite... POST <target> is to add an object and POST <target> + <id> is an incremental update. you don't need to inspect the object in order to get the id since it comes from the URL. in fact, what if you don't even include the id of the object in the partial data? :open_mouth:

requires in-depth knowledge of what HTTP requests the collection issues by default.

this is what an API is - you have clearly defined contracts that consumers and producers rely on being met.

Since dojo/request providers are registered globally, each instance of a collection using dojo/request interacts with global state.

URLs are global and since request providers are acting on URLs this is not a problem.

Overriding collection methods naturally provides per-instance encapsulation and does not rely on modifying global state

as i've more or less said earlier, you shouldn't have 2 collections representing the same target but if for some reason you do, then you shouldn't need per-instance overrides for the same target. (for cases like #81 i'm suggesting that store?objectTypeId=foo and store?objectTypeId=bar are different targets).

dojo/request providers need to be removed from the registry when your collection is no longer used, but dstore does not offer anything like a destroy method (and it could be difficult or impossible to know when all subcollections have fallen out of use). Unless the collection developer is conscious of this and provides their own solution, request providers will leak.

i'm yet to see a request provider that doesn't last the lifetime of an application. the behavior of external services doesn't typically change so you don't have a need to unregister a request provider.

in response to @Jens-dojo, i think you could be easily convinced to love request providers. what you describe is more or less what they are except that rather than be something that you pass to an instance of the store (e.g. this.requestProvider), you configure them in a more global sense and they intercept requests made via dojo/request which match some criteria (usually based on the URL but the mechanism is powerful enough for any condition to be used) - i.e. using request is already more or less what you're suggesting this.requestProvider should be.

Jens-dojo commented 9 years ago

Dear Ben,

I am still not convinced to use Global Request Providers instead of passing an individual Request Provider to a store as I suggested. Why should I work and why must I think on a global level, when I could have a well encapsulated Store-Request ensemble ? As soon as I work on a global Request Provoider level, I add a potential source of problems.

If I say "I want ALL XHR to be made as POST instead of GET", then yes, I use a Global Request Provider. Right place to do that. But if I have ONE store and I want the request to behave in a certain way, then I want ONE request provider directly attached to that store.

Why should I use a Global Request provider and have to analyse if the request is from that ONE specific store that needs special attention? If I have ten stores, I would need code in ONE request provider to sort things out between ten stores?

Code and system logic that belong together into one Object with one Store Object and one Request Provider are no longer separated from the Request Provider Logic of other stores?

Maybe I am still missing something when I think about the Global Request Providers ......?!

Best wishes Jens

neonstalwart commented 9 years ago

@Jens-dojo have you seen how you register a request provider with dojo/request/registry? I think doing that could be useful for informing your opinion.

I don't know if I can say much more without repeating myself or adding confusion. I think at this point I'd like to hear from others. the tl;dr of my position is that I agree that dstore could use a few more public methods for generating requests but I'm hesitant to include APIs which overlap with request providers.

cc @kfranqueiro, @bryanforbes, @csnover, @kriszyp

brandonpayton commented 9 years ago

Hi @neonstalwart,

@brandonpayton i wanted to respond directly to some of your comments but my responses are kind of terse and scattered which may make them seem hostile - don't take them that way, i'm just extremely busy right now. so first an attempt at a general response and then some specifics.

ok, understood.

My responses are inline.

URLs are an external dependency which are often not controlled by the person writing code that uses a dstore collection but the behavior of those URLs should be well-defined and consistent

in a "good" architecture, one collection is going to be the only thing interacting with a particular URL. i.e. you wouldn't get, update, or delete resources from a target without using the collection associated with those resources

This is why it is best to write customizations as a collection subclass. All logic for the customization is maintained in the subclass's module with no external side-effects.

you likely won't have 2 collections using the same target (although #81 is a really interesting case but _renderUrl would be enough to take care of that one) so you don't need instance-specific differences for 2 collections that have the same target.

Agreed. It is unlikely. My purpose in mentioning this was to show that subclassing collections is a better-encapsulated solution.

dstore/Rest makes unambiguous requests using dojo/request that completely convey the intent of the method called on the collection. these requests can easily be understood by a request provider without needing to have intimate knowledge of the resources. i would expect very few cases where it was not possible for a request provider to be able to adapt the request from a collection into a suitable HTTP request to an external data source.

I believe you're correct about Rest making unambiguous requests. My example of ambiguity was wrong. However, developers should be customizing collections at the collection level, not using an implementation detail of dstore/Rest.

some of these semantics can be changed by customizations to the collection but, given the bullet points above, it would not be a bad practice for the request provider and the collection to be working together rather than in complete isolation. as @Jens-dojo suggests it may be possible to build up a library of these request providers for well-known data sources and people could use a dstore/Rest instance combined with one of these request providers and be up and running without any further customization.

Why have to use an external thing in combination with an otherwise encapsulated collection? dstore should provide the API necessary to customize dstore. A user shouldn't have to learn that dstore/Rest is implemented using dojo/request and then learn how to properly use dojo/request/registry. I use request providers from time to time, usually in testing, but if I needed to customizing Rest requests, I would rather just override hooks and make the requests I want to make, not write a request provider to intercept undesired requests and convert them to desired requests. It's unnecessary thought and code.

A mixin overriding Rest hooks is more straightforward to write, read, and to use.

another concern i have with such fine-grained hooks is that it's veering into 2nd system syndrome. dojo/store had a very concise API but since dojo/request didn't exist at the time when the stores were written, they did not use dojo/request. introducing dstore/Request and using dojo/request should be sufficient to address the vast majority of issues that needed to be worked around with dojo/store and HTTP data sources.

I have had concerns about 2nd system syndrome with other parts of dstore, but I don't believe providing hooks for existing behavior enters that territory. The hooks do not force themselves upon the collection developer; they are simply available to override if desired.

In contrast, asking collection developers to customize Rest collections using dojo/request is forcing them to:

Learn more about an external package that is a dstore implementation detail
Always use dojo/request or at least use dojo/request/registry before using another request library of their choosing (e.g., a company where frontend developers are given a business-specific request library)

IMO, the biggest risk with the hooks is that we get the granularity wrong and make simple things hard to override without reimplementing other things. That said, I believe these hooks offer a reasonable level of granularity by allowing URLs and requests to be customized independently.

The methods ... represent the truth of what an HTTP collection needs to do for each operation.

the requests currently being made unambiguously represent the semantics of each operation. it's simple for a request provider to decode what is being asked for.

The collection API clearly, unambiguously represents the semantics of each operation. With hooks, there is no need to write the code to decode anything.

My biggest concern is that dojo/request providers operate at the level of an HTTP abstraction, not at a collection level.

this is actually exactly why they are the right place to do this - HTTP abstraction should be happening in the place that is responsible for doing HTTP requests. if i was only allowed one point in my argument, this would be it.

dstore is about the collection abstraction and encapsulating implementation details. Developers customizing dstore requests should be working with the collection abstraction as that is all they care about. If you're customizing requests, there's no need to think about the default HTTP request; just make the request you need based on arguments to the collection API.

Example: dstore/Rest issues a POST for both a put with options.incremental and a put for an object with no ID. To discern what operation the request is for, a request provider has to match on the HTTP method, parse the request body, examine the resulting object, have the knowledge of what field(s) constitute an ID in the collection, and use the presence of an ID as an indicator of a new object or a partial update of an existing object.

not quite... POST <target> is to add an object and POST <target> + <id> is an incremental update. you don't need to inspect the object in order to get the id since it comes from the URL. in fact, what if you don't even include the id of the object in the partial data? :open_mouth:

Yep. I was wrong about this one. I believe all Rest operations can be decoded from their HTTP request. I just don't think we should have to decode HTTP requests to send different HTTP requests.

requires in-depth knowledge of what HTTP requests the collection issues by default.

this is what an API is - you have clearly defined contracts that consumers and producers rely on being met.

There are two levels to this API: the collection API and the HTTP API. Those overriding the HTTP API are writing their own request contracts and shouldn't be required to know and couple their customization to the contracts they're overriding. They just need to understand the collection API because they are implementing a collection abstraction.

Since dojo/request providers are registered globally, each instance of a collection using dojo/request interacts with global state.

URLs are global and since request providers are acting on URLs this is not a problem.

It's better to avoid global constructs where possible, and it is possible here.

Customizing Rest using the request registry also introduces a defect vector: If a broadly-matching request provider (say, to slightly tweak all requests) is registered before a customizing request provider, the broadly-matching request provider will be called instead of the customizing request provider. One can argue that this is unlikely or probably the result of bad architecture, but what can happen will happen. And all of us make mistakes and do stupid things from time to time. Why not use a fully encapsulated solution that does not expose itself to these sorts of issues?

dojo/request providers need to be removed from the registry when your collection is no longer used, but dstore does not offer anything like a destroy method (and it could be difficult or impossible to know when all subcollections have fallen out of use). Unless the collection developer is conscious of this and provides their own solution, request providers will leak.

i'm yet to see a request provider that doesn't last the lifetime of an application. the behavior of external services doesn't typically change so you don't have a need to unregister a request provider.

The problem here is that collections are supposed to abstract away the underlying details, but your application now has to register request providers to modify collections' underlying behavior.

brandonpayton commented 9 years ago

@neonstalwart, I wanted to keep things simple and direct, but my response probably could have used the same warning about terseness.

kriszyp commented 9 years ago

I am not sure that I understand how the abstractions suggested by these new methods is really directly replacing dojo/request abstractions. Yes, they both deal with HTTP, but at a more nuanced level, there is a progression of computations that occur between a store level operation and the resulting network call, and granularizing the progression of these computations seems to fit reasonably well with typical programming recommendations of keeping methods small and simple with clear distinguished steps in their operation.

I am definitely attracted to minimizing our surface API, and if we can encapsulate a series of steps to minimize documentation and complexity, and retain flexibility for future changes, without any significant loss in extensibility, that would be great. However, I think what @brandonpayton has proposed does represent a pretty well-hardened set of steps for translating store operations to HTTP requests. The operational steps that would be exposed are basically the same steps that existed internally throughout dojo/store/JsonRest history, and have good backing in REST semantics and HTTP specification.

I wonder if there are particular methods proposed that would be more desirable to be cut? Perhaps the render*Url methods?

As far as the removal of the underscore, I feel like, from my experience, that the idea of using underscore's to denote "internal" (or "protected") instead specifically private (do not touch), has turned out to be more confusing and unhelpful. It becomes a vague combination of pushing some people away from using it (giving up the benefit of truly public) and luring others into using and making it prone to breakage for future changes (giving up the benefit of truly indicating that it is private). In fact, perhaps both these interpretations exist within this thread alone :).

kfranqueiro commented 9 years ago

Personally I would vouch for keeping the underscore prefix on these methods if they are to be included. They are not part of the public API per se, as you would never expect a store consumer to call any of them. At the same time, we should still have a section documenting them as methods intended to be extensible or overridable.

The fact that people assume underscore-prefixed to mean "private, never touch this" is somewhat unfortunate for cases like this, and I think we need to start making it clear that there are cases where it actually means "not meant to be called externally, but totally okay to extend". Though I suppose there is still the gray area of what degree of breaking changes are allowable in them in that case.

neonstalwart commented 9 years ago

@brandonpayton i don't want to stir this up again but in looking at #108 an extra point came to mind... if you have a server that requires you to adjust your request somehow (credentials, extra data, different method, etc) then using a request provider puts all of that for you in one place so that if some requests are going through dstore and others aren't then the request provider catches all of them for you.

it looks like this is such a common thing people are trying to figure out that whatever way you decide to go, i think it's probably something to move up the list of priorities for dstore so that users clearly understand how to achieve this. if request providers are the solution to recommend then it's only a matter of documentation - it would make a good FAQ blog post for sitepen.com :wink:

SitePen / dstore

Feature proposal / user-defined request parameters / pull request #79