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

Set anonymous functions display name using file name #37

Merged
merged 4 commits into from
Jun 11, 2018

Conversation

Ethan-Arrowood
Copy link
Member

Related to fastify/fastify#861

When a user adds a fastify plugin with an anonymous name, fp will use look up the stack trace and set the function objects display name to be the file name from which it was called. It stores this in a symbol: fp.DISPLAY_NAME_SYMBOL

For example:

// mySuperDuperPlugin.js
const fp = require('fp')

const fn = fp((fastify, opts, next) => {
  next()
})

console.log(fn[fp.DISPLAY_NAME_SYMBOL]) // 'mySuperDuperPlugin'

I've also added a unit test.
Regular expressions should work in Node version 6+ too

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work! I've left some nits.

index.js Outdated
if (fn.name.length > 0) return fn.name

const r1 = new RegExp('at\\s{1}plugin\\s{1}.*\\n\\s*(.*)')
const r2 = new RegExp('\\/(\\w*)\\.js')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move those two regexps at the top of the file and define them with more clear names?

index.js Outdated
@@ -3,11 +3,15 @@
const semver = require('semver')
const console = require('console')

const DISPLAY_NAME_SYMBOL = Symbol.for('display-name')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you name this fastify-plugin.display-name, to avoid any possible clash?

index.js Outdated
@@ -3,11 +3,17 @@
const semver = require('semver')
const console = require('console')

const DISPLAY_NAME_SYMBOL = Symbol.for('fastify-plugin.display-name')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually this is better as Symbol.for('fastify.display-name'), and we look for it in fastify/fastify#861, and we document it as part as https://github.com/fastify/fastify/blob/master/docs/Plugins.md.

I would not export the symbol from this module then.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Ethan-Arrowood
Copy link
Member Author

😁 I'll start integrating this with toString

Copy link
Member

@delvedor delvedor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

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

Successfully merging this pull request may close these issues.

None yet

3 participants