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

Fix plugins support #3

Merged
merged 1 commit into from Sep 3, 2015
Merged

Fix plugins support #3

merged 1 commit into from Sep 3, 2015

Conversation

RobLoach
Copy link
Member

@RobLoach RobLoach commented Sep 2, 2015

On line 12, plugins was a null variable, so no plugin was ever used:

var plugins = opts.plugins && Array.isArray(plugins) ? plugins : [plugins];

plugins undefined.

This will fix it, and no longer require the .filter call.

@tunnckoCore
Copy link
Member

It won't allow to pass just single string as plugin, when only one plugin is needed. That's why didn't do it like that.

@RobLoach
Copy link
Member Author

RobLoach commented Sep 2, 2015

It's called plugins, which is plural. One would assume it would just take an array of plugins.

@tunnckoCore
Copy link
Member

It's called plugins, which is plural.

Yes, right. But i think it also should accept string. We all can agree that we have tools that works in that way and it's normal to "just work" if someone pass just string - it's would be expected to work.

@ForbesLindesay
Copy link
Member

The code before doesn't work at all in any case, so I'm merging this for now. Two things that can be discussed separately:

  1. Should it accept a single item, rather than forcing you to create an array of one item? If not, it should probably error, rather than silently ignoring this case.
  2. Should it accept strings, as an alternative to the actual plugin modules. I believe we do this with jstransformer-less.

ForbesLindesay added a commit that referenced this pull request Sep 3, 2015
@ForbesLindesay ForbesLindesay merged commit 17110dc into master Sep 3, 2015
@ForbesLindesay ForbesLindesay deleted the fix-plugins branch September 3, 2015 12:01
@tunnckoCore
Copy link
Member

The code before doesn't work at all in any case, so I'm merging this for now.

yea, noticed that before few weeks

Should it accept a single item, rather than forcing you to create an array of one item?

👍 yes, it should. And we should apply this strategy for all transformers that have "plugins" support.

@RobLoach
Copy link
Member Author

RobLoach commented Sep 3, 2015

#4
#2

RobLoach pushed a commit that referenced this pull request Jun 12, 2016
feat(inputFormats): Add inputFormats
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