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

A DRYer way to import functions #6

Open
djfarrelly opened this issue Nov 26, 2018 · 3 comments
Open

A DRYer way to import functions #6

djfarrelly opened this issue Nov 26, 2018 · 3 comments
Labels
enhancement New feature or request

Comments

@djfarrelly
Copy link

djfarrelly commented Nov 26, 2018

Extracting an idea from https://github.com/bufferapp/core-authentication-service/issues/15, what if we had a DRYer (don't repeat yourself) way to import a bunch of RPC functions?

Right now, the way we're doing things, we've set up projects to require 3 pieces in place:

  1. The rpc index file - ./src/rpc/index.js (example):
module.exports = {
  createAccount: require('./createAccount'),
  getAccount: require('./getAccount'),
}
  1. The require of all functions - usually in the main Express server's index.js file (example)
const {
  createAccount,
  getAccount,
} = require('./rpc')
  1. Registering the rpc functions (example)
  app.post(
    '/rpc',
    rpc(
      method('createAccount', createAccount),
      method('getAccount', getAccount),
    ),
    errorMiddleware,
  )

3.5. BONUS - Why do we need to use method('methodName', methodName)?


Question: - Why do we need to repeat ourselves so much?
Idea - Can we remove most of these steps by providing a simpler way to do this?

// idea 1
// we remove the ./rpc/index.js file and dynamically require all files in that directory
// if a string (path to files) is the first argument 
const { rpc, errorMiddleware } = require('buffer-rpc')

// Even this line of code says `rpc` 3 times 😁
app.post('/rpc', rpc('./rpc'), errorMiddleware)

// idea 2
// same as idea 1, but we use a load function to require all the rpc functions
const { rpc, loadFunctions, errorMiddleware } = require('buffer-rpc')

const functions = loadFunctions(path.join(__dirname, './rpc'))
app.post('/rpc', rpc(functions), errorMiddleware)

// idea 3
// make the error middleware automatic if you're already using `rpc()`
const { rpc } = require('buffer-rpc')

app.post('/rpc', rpc('./rpc'))

// idea 4
// what if we just assumed the rpc directory was the default arg?
const { rpc } = require('buffer-rpc')

app.post('/rpc', rpc())
@esclapes
Copy link
Contributor

Adding my two cents here for @philippemiguet and for transparency 😄

I agree that current convention adds a bit of boilerplate to the index.js, and I am 💯 in favor of DRYing it if possible.

I am sharing an alternative approach that also address points 2 and 3 form the OP. It does not really removes all the boilerplate but packs code a bit 😁

  1. Move all buffer-rpc logic to ../rpc/index.js and export the handler
import { rpc, method } from 'buffer-rpc'
import createAccount from './createAccount'
import getAccount from './getAccount'

module.exports = rpc(
  method('createAccount', createAccount),
  method('getAccount', getAccount),
)
  1. Use it in app.js (or index.js) as any other express router
import rpcRouter from './rpc'

app.post('/rpc', rpcRouter)

With this approach, all the wiring is explicit, but the app.js does not need to be touched when adding new methods, just the ./rpc/index.js needs a couple of extra lines.

The dynamic loading that Dan suggested would, of course, make this even more DRY, but also a bit more magic and require a (breaking?) buffer-rpc release.

@djfarrelly djfarrelly added the enhancement New feature or request label May 14, 2019
@djfarrelly
Copy link
Author

Riffing on the approach here, we could push a 2.0 version that might enforce every package in a directory to wrap their function in method prior to exporting. For example:

// /rpc/createAccount.js
const { method } = require('@bufferapp/rpc')

module.exports = method(
  'createAccount',
  'Create a new global Buffer account',
  async ({ email, password }) => {
      const account = new Account({ email, password })
      await account.save()
      return account.toJSON()
  }
)

This would allow us to still use the method function for the documentation functionality, although underused now we could automatically generate API docs.

@philippemiguet
Copy link
Contributor

philippemiguet commented Sep 12, 2019

@djfarrelly, that's a great intermediary step, I've seen engineers doing that already in different repositories 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants