Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add processor plugin #1741

Merged
merged 4 commits into from Jan 14, 2018
Merged

Add processor plugin #1741

merged 4 commits into from Jan 14, 2018

Conversation

ricardograca
Copy link
Member

@ricardograca ricardograca commented Jan 12, 2018

Introduction

Adds Attributes Processor plugin. It allows defining custom processor functions that handle transformation of attribute values whenever they are .set() on a model.

It also adds documentation to the Bookshelf#plugin() method which was totally absent.

There is also a new wiki page for this plugin at Plugin: Processors.

Motivation

This seems like a worthy addition and was also mentioned as part of #552. Adding it as a plugin for now like was mentioned in issue #877.

Proposed solution

Mostly based on the code proposed by @rhys-vdw on this comment. The Cast Plugin proposed in the above linked PR was a bit too specific in terms of naming conventions and didn't support having pre-built processors (or cast functions using that plugin's nomenclature) that could be assigned to specific attributes by string name. It was otherwise functionally identical to the code proposed by @rhys-vdw.

On the other hand, the code proposed by @rhys-vdw implied having some processors already in Bookshelf, but that seemed like an unnecessary inclusion. These processors are probably specific to the user's application and can be converted to standalone npm packages or community plugins if needed with the present solution.

The solution proposed in #1220 may seem different from this at first glance, but it's actually trying to achieve the same thing. The problem with that solution is that it's too complicated since it tries to attach the functionality to the Virtuals plugin which means it had to come up with workarounds for things like the possibility of infinite recursion.

Fixes #809.
Closes #1220.
Closes #591.
Ticks the box for missing documentation of Bookshelf#plugin on #800.

Alternatives considered

Considered merging PR #591, but it was a little bit too specific about casting values, and didn't include as much documentation as the current PR.

Current PR Issues

There are several ways of adding processors right now. Maybe the [add|remove]Processor methods aren't that necessary since it's still possible to reuse code if users just keep them in a separate module and require() it as necessary in models. This also implies that the ability to assign attribute processors by string name would become irrelevant as well. One could just do:

// In a model file
var myProcessors = require('./lib/processors') // or require('some-npm-package')
var MyModel = bookshelf.Model.extend({
  tableName: 'stuff'
  processors: {
    username: myProcessors.trim
  }
})

This PR also adds a new Wiki page instead of moving the plugin wikis to JSDoc.

@ricardograca ricardograca added this to To Do in Version 0.13.0 via automation Jan 12, 2018
@ricardograca ricardograca moved this from To Do to In Progress in Version 0.13.0 Jan 12, 2018
@ricardograca
Copy link
Member Author

No idea why Travis is failing on Node 4 right now on the pr build. Seems to be related with installing Oracle, although the error is about dpkg not being able to obtain a lock. Will have to be investigated if it persists.

@ricardograca ricardograca merged commit ae679d5 into master Jan 14, 2018
Version 0.13.0 automation moved this from In Progress to Done Jan 14, 2018
@ricardograca ricardograca deleted the rg-processor-plugin branch January 14, 2018 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

1 participant