Extensions! #146
Extensions! #146
Conversation
lib/index.js
Outdated
| * @param {Plugin} plugin - A plugin with at least one hook implemented | ||
| */ | ||
| ext(plugin) { | ||
| Object.keys(HOOKS).map((key) => HOOKS[key]).forEach((hook) => { |
There was a problem hiding this comment.
Better, I was looking for this but got stuck with forof loop
test/index.js
Outdated
| onWrite: (input) => input.replace('b', 'd') | ||
| }); | ||
| return unifile.writeFile({}, 'memory', '', 'ab') | ||
| .then(() => expect(inMemoryFile).to.equal('db')) |
There was a problem hiding this comment.
shouldn't it be... dd (ab => first onWrite (a to b) => bb => second onWrite (b to d) => dd)
There was a problem hiding this comment.
I though that to but the replace is not global, so only the first match is replaced.
There was a problem hiding this comment.
haha wtf
then maybe you should put a more obvious transformation in your test, because the test is pretty tricky to read (maybe upper case / lower case, prefixing / unprefixing, ...)
also I was wondering: is input just a chunk, or is it garanteed to be the full content ? if it's a chunk, then maybe you should think about implementing the whole stream API
test/index.js
Outdated
| .should.eventually.equal('aa'); | ||
| }); | ||
|
|
||
| it('follow a chain of action in order', function() { |
There was a problem hiding this comment.
IMHO it shouldn't be possible to add several codecs, as the spec is very confusing:
For instance, if you use 2 extension, say cipher / decipher and then zip, on write it's all fine (cipher then zip), but on read, it would certainly break (decipher then unzip => boom)
I think you should either clarify the spec (and likely fix your implementation :S) or allow only 1 ext / hook.
I think allowing only 1 ext is better anyway, since you can still compose your onRead / onWrite hooks like onWrite: compose( cipher , zip ), onRead: compose( unzip, decipher )
samples/extensions.js
Outdated
| const Unifile = require('../lib/'); | ||
| const unifile = new Unifile(); | ||
|
|
||
| const cipher = crypto.createCipher('aes192', 'a password'); |
There was a problem hiding this comment.
duplicate => constants :D const PASSWORD = 'a password'
|
@clemos Fixed as you requested Bonus, I added plugins for stream 😃 |
There was a problem hiding this comment.
I like how simple this is to create a plugin (impressive the "encrypt" plugin!
Maybe the plugins should have a ON_BEFORE_* and be able to change all the input data?
And will it be a limitation that we can only have 1 set of plugin? I mean what if Silex calls unifile.ext and CE did too before it? Or what if I have an object which is the "encrypt" plugin and another which is the "change extensions" plugin, and I want to activate one or the other depending on my project settings...
| @@ -258,7 +302,8 @@ class Unifile { | |||
| * @return {external:Promise<string>} a promise of the content of the file | |||
| */ | |||
| readFile(session, connectorName, path) { | |||
| return this.callMethod(connectorName, session, 'readFile', path); | |||
| return this.callMethod(connectorName, session, 'readFile', path) | |||
| .then((content) => applyPlugin(this.plugins[HOOKS.ON_READ], content)); | |||
There was a problem hiding this comment.
here maybe the plugins should have a ON_BEFORE_READ in order to change the paths and possibly the session and connectorName
Thx to @clemos
Also now only one plugin is available per action.
Create a basic system of extensions on read and write action. This will be adaptable to be able to cover a lot of hook points in the future.