dojo / core

:rocket: Dojo 2 - language helpers and utilities.
http://dojo.io
Other
213 stars 58 forks source link

Simplify core/request #253

Closed rorticus closed 7 years ago

rorticus commented 7 years ago

Currently the request method will dynamically load the default provider, depending on if you are running in node or the browser. This adds some complexity to the code and probably isn’t even needed.

We’re looking for a usage like this,

import request from 'dojo-core/request';
// import node support manually if we want to use it
import nodeProvider from 'dojo-core/request/node';

// set the default provider some how
request.setDefaultProvider(nodeProvider);

// the next request will go to node
request.get(...);

You can see a rough implementation (as well as some unrelated changes...) here,

https://github.com/dojo/core/compare/master...rorticus:no-streams

sebilasse commented 7 years ago

Just my 2 cents : I like the dynamic loading. It is consistent with all modules which you can use either in the browser or node.js ... e.g. /text, /util, /queue, /on have switches for node/browser and as a user I would expect to use any module without external requirements.

matt-gadd commented 7 years ago

@sebilasse We would still support dynamic loading (even when in most environments it is definitely more desirable to make concrete at build time), it just wouldn't happen in the core of request itself. Currently the code to dynamically require down the browser path is super ugly - we queue requests while loading the provider, this kind of thing doesn't seem like it should be a request concern.

matt-gadd commented 7 years ago

Following on: In the past, the above complications would've been removed by dealing with this in the dependency block in AMD using the has plugin to conditionally require the environment specific module. But given we're consciously trying to keep as format neutral for the future we're better placed on making these kind of module loading decisions outside of request. The point being that the environment will/should be known prior to a request being made.

rorticus commented 7 years ago

Fixed in #261