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

Plugin options #1833

Closed
jmm opened this issue Jun 25, 2015 · 23 comments
Closed

Plugin options #1833

jmm opened this issue Jun 25, 2015 · 23 comments
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue
Milestone

Comments

@jmm
Copy link
Member

jmm commented Jun 25, 2015

I feel like plugins are going to be pretty hamstrung without an interface for passing options to them. There's nothing documented for that and as far as I know it doesn't currently exist. If that's correct, perhaps 6.0 would be a good opportunity to introduce this.

Example:

babel.transform(code, {
  // How to provide configuration options to `whatever`?
  plugins: ['whatever'],
});

Thoughts?

@sebmck
Copy link
Contributor

sebmck commented Jun 25, 2015

Yeah, I talked about this a bit with @DmitrySoshnikov. The only way plugin options could be implemented is if the fourth visitor method parameter was a separate file and plugin-specific instance. Unfortunately changing that would be a breaking change (even though it's never actually documented anywhere, people are actively using it...) so yeah, it'll have to wait for 6.0.0.

@sebmck sebmck added this to the 6.0.0 milestone Jun 25, 2015
@DmitrySoshnikov
Copy link
Contributor

Yeah, for now we've been using extra option which is shared between all plugins (this approach works fine actually). You may also namespace needed options by plugin name there if you want:

extra: {
  whatever: {
    planB: true,
  }
}

And then access:

state.opts.extra.whatever.planB;

@jmm
Copy link
Member Author

jmm commented Jun 26, 2015

Thanks for the replies.

@DmitrySoshnikov So are you saying you're satisfied with that as a permanent solution and don't mind plugins having access to each other's options? Or do you think it should be tightened up in the way @sebmck described when BC can be broken? @sebmck, would you be satisfied with plugins having access to each other's options?

I don't particularly care about the CLI, but I know others do. Something additional would be needed to accomodate plugin options via the CLI, right?

How about babelrc -- should you be able set plugin options via that?

@sebmck
Copy link
Contributor

sebmck commented Jun 26, 2015

would you be satisfied with plugins having access to each other's options?

Not sure. Would probably need to group the options somehow in a similar vain to the visitor merging on metadata.group.

How about babelrc -- should you be able set plugin options via that?

Plugin options could be set via .babelrc in similar vain to browserify and eslint:

{
  "plugins": [
     ["plugin-name", { "options": "here" }]
  ]
}

I don't particularly care about the CLI, but I know others do. Something additional would be needed to accomodate plugin options via the CLI, right?

CLI would work the same as Browserify:

$ browserify -t [ babelify --option-here ] script.js

@jmm
Copy link
Member Author

jmm commented Jun 26, 2015

@sebmck

Plugin options could be set via .babelrc in similar vain to browserify and eslint:

Ok, so if that's going to be the case I think that's how the API should work also (as opposed to passing the options via extra in the API).

CLI would work the same as Browserify:

👍 That's what I was going to suggest.

@RnbWd
Copy link

RnbWd commented Jul 8, 2015

postcss has a set of guidelines for writing plugins which I find interesting. The config @sebmck proposed looks solid:

{
  "plugins": [
     ["plugin-name", { "options": "here" }]
  ]
}

Browserify's API uses plugin/transform methods:

var b = babel(); 
b.plugin('whatever', {options: 'here'}) 

@jmm 's original example, could be expressed like this:

var whatever = require('whatever');
babel.transform(code, {
  // options passed into plugin as params
  plugins: [whatever({options: 'here'})],
});

This syntax is fairly common, and postcss is particularly interesting because the plugins interact with one another, moreso than with gulp or even browserify.

@jmm
Copy link
Member Author

jmm commented Jul 10, 2015

@RnbWd

@jmm 's original example, could be expressed like this...

(The ability to pass a function instead of a string isn't even documented yet.) I think to do it that way you'd have to use a different function that takes an options object and returns a function that takes a "babel" object, whereas the main export would need to be a function that takes a "babel" object and options object in order to support referencing the plugin by string. E.g.:

export default function (opts) {
  return function (babel) {};
};

// vs.

// For referencing by string
export default function (babel, opts) {};

So it should probably support:

plugins: [
  [someFunc, {options: 'here'}]
]

Unless the API changes to: export a function that takes an options object and returns a function that takes a "babel" object for both cases.

There's also the matter of what's going to become of the :before|:after designation for plugins and how that will be specified when passing a function (if applicable).

@phpnode
Copy link
Contributor

phpnode commented Jul 15, 2015

I've been using environment variables for this, but it's not ideal. It does however have the benefit that it's trivial to override options from the CLI which is very useful. For instance, I'm writing a tracing plugin which allows something like this:
routes/foo.js

export function someRoute (req, res) {
  trace:info: "handling a request for `someRoute`";
  const result = this.handle(req, res);
  if (result instanceof Error) {
    trace:error: "Error in someRoute", result;
  } 
  return result;
}

In my .babelrc it'd be nice to be able to set something like:

{
  "plugins": [
    {"name": "trace", "options": {"levels": ["error"]}}
   ]
}

So that by default, only error messages get traced. But if I'm debugging, I don't want to have to edit .babelrc, I want to be able to override via the CLI, either via environment variables:

BABEL_PLUGIN_TRACE_LEVELS=info,error npm test

or via command line options

babel --plugin-trace-levels=info,error ./src

It'd be really nice to have a standard way to do this.

@RnbWd
Copy link

RnbWd commented Jul 17, 2015

Unless the API changes to: export a function that takes an options object and returns a function that takes a "babel" object for both cases. @jmm

Hopefully I can clarify what I meant using some real-world examples. In babelify's readme, the author encourages the use of this syntax:

// var browserify = require('browserify'), babelify = require('babelify')
browserify().transform(babelify.configure({
  optional: ["es7.asyncFunctions"]
}))
browserify -d -e script.js -t [ babelify --optional es7.asyncFunctions ]

Yet, browserify's handbook uses this example:

var b = browserify('main.coffee');
b.transform('coffeeify');
browserify -t coffeeify main.coffee > bundle.js

In .json config files (which are almost identical in babel & browserify for this), you're limited to strings, arrays, and objects.

In reality, almost all plugins are individual npm packages, which are either required internally and in your projects main package.json. So even though you're declaring plugins as strings - they're actually npm packages which are required then parsed as functions somewhere in code. Theoretically it shouldn't make a difference whether a string or function is passed into the plugin array (as long as the plugin is compatible with babel).


Honestly I'm not well versed with the specifics of how `babel` handles plugins... but I'm very familiar with the architecture of `browserify` and `postcss` - I assume it's similar. 

(side note: [new-struct](https://github.com/azer/new-struct) would take care of passing in `babel` objects, although I wouldn't suggest using it, I'd def recommend reading the docs) 

@sebmck
Copy link
Contributor

sebmck commented Jul 17, 2015

Currently, babel explicitly requires strings to be passed as plugins.

This is wrong. Babel can take plugin instances AKA functions too.

@RnbWd
Copy link

RnbWd commented Jul 17, 2015

@sebmck Awesome!! I misinterpreted what @jmm said above and misinterpreted this in the source:

export default class Plugin {
  constructor(key: string, plugin: Object) {
    Plugin.validate(key, plugin);

but then I saw this:

    if (typeof plugin === "function") {
      plugin = PluginManager.memoisePluginContainer(plugin);
      plugin.key = key;
      plugin.metadata.optional = true;

so obviously plugins can be functions

I'll just scratch that out... 😝

@jmm
Copy link
Member Author

jmm commented Jul 17, 2015

@RnbWd There are a number of ways to approach this, but I think the most straightforward baseline is to just have this:

plugins: [['some-func', {/* opts */}]]
// and
plugins: [[someFunc, {/* opts */}]]

Theoretically it shouldn't make a difference whether a string or function is
passed into the plugin array (as long as the plugin is compatible with babel).

Here is the difference I'm talking about:

whatever.js

module.exports = function (babel, opts) {
  babel.Plugin(/* ... */);
};

build.js

babel.transform(code, {
  plugins: [['whatever', {some: 'thing'}]]
});

So far so good. But then you can't just do this without accounting for it somehow:

plugins: [require('whatever')({some: 'thing'})]

You can add extra logic to the plugin function to handle it, or provide a different method, like plugins: [require('whatever').cfg(...)], for the API, or as I mentioned make it work this way for both cases:

module.exports = function (opts) {
  return function (babel) {
  };
};

But I'm not sure what the advantage would be of doing any of those things instead of making the baseline what I suggested as the top (pass an array of [modNameOrFunc, opts] for each plugin.

(side note: new-struct would take care of passing in babel objects, although I wouldn't suggest using it, I'd def recommend reading the docs)

Hm?

babel accepts functions as plugins but I'm not sure if documented

Last I checked it was not, but I've heard new plugin documentation is in the pipeline.

@RnbWd
Copy link

RnbWd commented Jul 19, 2015

@sebmck responded to my comment above the previous one:

This is wrong. Babel can take plugin instances AKA functions too. - @sebmck

I support this syntax:

plugins: [['some-func', {/* opts */}]]

not this syntax:

plugins: [[someFunc, {/* opts */}]] // how are options passed into the function?

yet in browserify, these are equivalent:

plugins: [require('whatever')({some: 'thing'})]
// or
plugins: [['some-func', {/* opts */}]]

I use both syntaxes 50/50 in browserify depending on (my mood?) and in some rare cases where the plugin is required as a subset of a module like require('whatever/plugin'). I don't expect any changes in the syntax for plugin modules themselves - it's just a behavior I expect based on similar projects.

@sebmck
Copy link
Contributor

sebmck commented Jul 20, 2015

A module that exports a function that returns an function is pretty hacky and a unique Plugin instance for each set of options is not going to be very efficient. The options are likely going to be localised to some other sort of instance that's passed to a Plugins visitor methods.

@jmm
Copy link
Member Author

jmm commented Jul 20, 2015

@RnbWd

sebmck responded to my comment above the previous one:

I know it can do it, the documentation still only covers strings though. sebmck's comments do make it sound as though passing a function will be supported and documented though.

I support this syntax:

plugins: [['some-func', {/* opts */}]]

not this syntax:

plugins: [[someFunc, {/* opts */}]] // how are options passed into the function?

The options are passed the same way as in the other case: in either case the function exported by the module accepts an options argument:

// Same function in both cases

plugins: [['some-plugin', {/* opts */}]]
// or
plugins: [[require('some-plugin'), {/* opts */}]]

yet in browserify, these are equivalent:

plugins: [require('whatever')({some: 'thing'})]
// or
plugins: [['some-func', {/* opts */}]]

Those aren't equivalent by default (if both cases refer to the same module). The plugin would need to implement some internal logic to make those behave equivalently. Far more likely these are equivalent:

plugin: [['some-plugin', {/* opts */}]]
// or
plugin: [[require('some-plugin'), {/* opts */}]]

@RnbWd
Copy link

RnbWd commented Jul 21, 2015

commonjs returns functions (which could return a string and make the latter equivalent) - currying functions - if done cleanly - isn't messy. There's nothing wrong with passing functions down the plugin pipeline - it's common practice. It's not much different than passing a string.

I have no idea how the plugin: [[require('some-plugin'), {/* opts */}]] syntax could be implemented in a sane way,,,- not meant as a critique - it just seems like a mess of code (unless you know some JS secret sauce I ain't aware of - in which case I'd be impressed)

@jmm
Copy link
Member Author

jmm commented Jul 21, 2015

@RnbWd

commonjs returns functions (which could return a string and make the latter equivalent)

In this example that would just make a broken module.

I have no idea how the plugin: [[require('some-plugin'), {/* opts */}]] syntax could be implemented in a sane way,,,- not meant as a critique - it just seems like a mess of code (unless you know some JS secret sauce I ain't aware of - in which case I'd be impressed)

I'm not sure what seems so different about the two forms to you. It's just requiring the plugin module sooner or later. plugin: ['some-plugin', {/* opts */}]] is just shorthand for "require 'some-plugin' for me and call the function it exports" and primarily exists for the sake of CLIs and config files. For browserify a plugin module should look like this:

module.exports = function (b, opts) {};

It's loaded and invoked something like this:

if (typeof plugin === 'string') plugin = require(plugin);
// Where `opts` are the opts you passed earlier
plugin(b, opts);

@sebmck
Copy link
Contributor

sebmck commented Sep 27, 2015

Added.

@sebmck sebmck closed this as completed Sep 27, 2015
@hzoo
Copy link
Member

hzoo commented Sep 27, 2015

party! 👯

@jmm
Copy link
Member Author

jmm commented Sep 28, 2015

Awesome, thanks!

@NekR
Copy link
Contributor

NekR commented Sep 29, 2015

Added.

Added where? Any info, commits, docs?

@jmm
Copy link
Member Author

jmm commented Sep 29, 2015

@NekR I assume he means on the development branch for release in v6.

@NekR
Copy link
Contributor

NekR commented Sep 29, 2015

@jmm hmm.. thanks! I thought v6 development goes in 6.0 branch, missed development one.

@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jul 11, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Jul 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue
Projects
None yet
Development

No branches or pull requests

7 participants