search

poetic / drupal-services-api

A Node.js client for interacting with Drupal 7 Services
14 stars 4 forks source link

Webpack and Browserify compatibility #4

Closed sebas5384 closed 9 years ago

sebas5384 commented 9 years ago

Hey there! we are using this module, and it's being really well :+1: , till now.

I was thinking why not using superagent with promises instead, enable us to be more compatible with Webpack or Browseriy right out of the box. Today in order to make it work you have to do some tweeks exclusively because this module uses request lib, which is not well prepared for this case. Other thing is that, superagent uses the middleware paradigm which is more light and stateless, so extending the request shouldn't be difficult and don't need to carrie around an instance of an object (Drupal).

Promises are really cool, so there is a module superagent-bluebird-promise which we could use.

I'm currently working on this, so I would love to know any thoughts about this.

Cheers!

MatthewHager commented 9 years ago

Sounds awesome. We are working on a large site that requires a bunch of data from 3rd party sources. If you submitted a tested PR and there are no ill side effects, we'd accept and continue to support it. @DanielOchoa: thoughts?

sebas5384 commented 9 years ago

good to know @MatthewHager !! thanks.

We are using this module in a project with an isomorphic ReactJS Flux stack with Webpack and Drupal Headless as an API, so should be a good to count on a good Drupal client :)

For example, for Drupal 8 we could think implementing a hal+json middleware for superagent.

DanielOchoa commented 9 years ago

@MatthewHager superagent definitely looks more tested, popular and developed than request lib. It also works with browserify and webpack. This would be a favorable change indeed.

sebas5384 commented 9 years ago

great then!! I'll be pushing changes to a pull request in the next days :)

sebas5384 commented 9 years ago

Till now we have 3 different approaches:

  1. Superagent middleware for Drupal authentication (CSRF and Cookie).
  2. Doing a wrapper around Superagent with the methods like .login(), node.index() or user.create().
  3. New approach using the lib Restful.js instead of superagent.

I tried the 1s one but didn't go well, because middle wares has only access to the request and you have make .use() every time before a request, so I jumped to the 2nd, but making a brainstorm with @lucasconstantino (he haves good Angular experience) show me a new lib called Restful.js which is the "rest" part of a well known lib (in the Angular's world) called Restangular. Resful.js is also compatible with Webpack and Browserify.

Given I liked the Restful.js paradigm and is being used with ReactJS and Flux, approach number 3 seems to be the way to go.

Sorry about the change of mind, but now I can't convence me that superagent is the better way to go. Let's change the issue's title?

[UPDATE] Example: https://github.com/marmelab/react-admin

sebas5384 commented 9 years ago

Since there's an issue about Isomorphic apps using Restful.js we are gonna have to go with the superagent wrapper, maybe next time.

sebas5384 commented 9 years ago

We made some good progress and we add some other methods like system/connect and user/token but with some automation.

All the tests are passing, but some differences with the current version, for example we are returning the entire response object which haves the resultant JSON decoded in the body response.body.

Some cleanup in order to pass the jshint, but it's no impediment to have a look at the branch https://github.com/TallerWebSolutions/drupal-services-api/tree/request-browser-compatible

Still there are some refactoring needed yet, but it's fully working! :+1:

DanielOchoa commented 9 years ago

Good to hear! Feel free to send a pull request this way whenever you feel its ready.