kristianmandrup / authorize-mw

Authorization middleware for Node.js and Javascript platform in general (with a little twist)
MIT License
8 stars 0 forks source link

Questions About Dependencies #7

Open digitalplaywright opened 10 years ago

digitalplaywright commented 10 years ago

Congrats with the release of authorize-mw! If you are interested I want to continue our discussion on dependencies from the Ember Simple Auth tracker.

I have some question about if some of the dependencies can be reduced:

  1. model-mw
    • It seems like this dependency is only used in https://github.com/kristianmandrup/authorize-mw/blob/master/lib/mw/authorize_mw.ls
    • If so, would it be appropriate to create sub-projects that depend on authorize-mw and assume an object model like e.g one of the many angular object models, model-mw or Ember Object? I think helper functions that can't be generic would be beneficial for each of these contexts at any rate.
  2. middleware
    • Does not seem to be used anywhere, is that correct? Maybe it is related to the model-mw dependency.
  3. lodash
    • As far as I can see only extend and merge from lodash is used now. Is this correct?
  4. sugar
    • I could only find a usage of unique from this library. Is this correct?
  5. prelude-ls
    • I don't know LiveScript well enough so some syntax that I think comes from LiveScript might actually come from prelude-ls. But where is prelude-ls constructs currently used in the library and can it be rewritten to pure LiveScript?

Making the project more self-contained and smaller makes it easier to contribute and understand the code. It might also help adoption since it becomes more palatable to more javascript sub-communities. Which is great, since people need to stop reinventing this particular piece of functionality for every app. :)

Thank you again for releasing this library. Having access to cancan-like functionality client-side is potentially very powerful.

kristianmandrup commented 10 years ago

Hi Andreas,

I certainly agree with all your points. Those dependencies are really not all that necessary. Basically it just used them for convenience to move ahead. I always plan for various "cleanup phases" as a project stabilizes. I actually removed 2 npm dependencies just this morning for similar reasons. Yes, both lodash and sugar could easily "go away" if that is a concern.

The middleware part is simply due to the fact, that it was originally targeted for use with a middleware layer, to be used with Racer.js Real Time sync engine, which also lacks proper validation/authorization at this time.

We could easily extract the "core" og authorize-mw into another npm such as "permit-authorizer".

preludels is the lodash/underscore of LiveScript, but I mostly used `.is-typewhich is ugly and better rewritten as core LiveScripttypeof! x is 'String'` so that could go away too...

I will try to release a permit-authorizer module tmrw and have the authorize-mw use it as a core dependency. Much better I agree ;) Then I/we need to test usage of the permit-authorizer using javascript and coffeescript. Thanks for your interest! Cheers!

PS: There is initial support for defining the permits as json structures that are loaded... to be improved ;) See wiki (I think?)

digitalplaywright commented 10 years ago

That sounds like a great plan. Thanks!

Btw, is it also possible to remove util as a dependency? Unless I missed something only the inspect method is used. Together with all the dependency removals in your last message permit-authorizer would then be fully self-contained, and that would be awesome!

kristianmandrup commented 10 years ago

Agreed, I only use util for better error messages ;-)

kristianmandrup commented 10 years ago

I released my first version of https://github.com/kristianmandrup/permit-authorize without the -mw dependencies. Now feel free to fork the project, remove the other deps and make a pull request ;) Then we could perhaps make an ember version of this project which also plays nice with your ember-simple-auth :)

kristianmandrup commented 10 years ago

started removing dependencies in this branch https://github.com/kristianmandrup/permit-authorize/tree/remove-deps

kristianmandrup commented 10 years ago

I minimized the dependencies to only lodash now. A few tests fail, needs a little debugging tmrw.

digitalplaywright commented 10 years ago

That is great! I really look forward to a version without external dependencies both because it will be easier for me and others to understand the code, and it will also be smaller.

Btw, I am not familiar with LiveScript modules and module exporting/require as well as lodash merge and extend. Is this equivalent to es6 modules together with export and import functions?

kristianmandrup commented 10 years ago

module.exports is simply standard CommonJS module functionality as used by node. The class syntax used is simply syntax sugar to create a constructor function where the keys of the object becomes the prototype functions. A Livescript module is simply a mixin that when used with extend is merged onto the class or module extending (like in Ruby et. al). Lodash merge and extend are simply used to merge two objects, shallow or deep copy. Nothing special. No magic ;)

digitalplaywright commented 10 years ago

I see. I am new to node and I went straight to using ES6 modules with a transpiler for backwards compatibility. Others might be doing the same. Good to know that lodash merge and extend is just another implementation of deep and shallow copy (there are one in so many libraries!).

Are you still planning to remove lodash as well? A quick search through the code-base shows that deep copy is only used in access-obj on https://github.com/kristianmandrup/permit-authorize/blob/17213f6dcf5c57dad7f616dfb57b9fca5fcc6f4b/lib/ability.ls#L16 and that function is never called, so maybe removing lodash is just about implementing shallow copy.

kristianmandrup commented 10 years ago

Just do a search for "lo." then you will see all the usages. Mostly I use some key Array functions and lo.keys. I'm not planning to remove lodash, so if that is critical to you, please go ahead and fork, make the changes necessary and send me a pull when you are done.

Looks like this could come in handy ;)

http://lodash.com/custom-builds

Use the include command to pass comma separated names of functions to include in the build.

lodash include=each,filter,map

Just make a build of the functions used and drop it in as "lodash-lite.js" and change requires to:

lo = requires.util 'lodash-lite'

Sweet as cake!

kristianmandrup commented 10 years ago

lodash functions used:

kristianmandrup commented 10 years ago

I made a lodash-lite minified build and used it in place of lodash. Now it has no dependencies. Self-contained ;) Time to move on...