millermedeiros / esformatter

ECMAScript code beautifier/formatter
MIT License
970 stars 91 forks source link

Consider passing the esformatter instance to plugins #348

Closed royriojas closed 8 years ago

royriojas commented 9 years ago

I know this might sound a bit weird, because calling the main module from the plugin seems to defeat the purpose of plugins

But here is the use case:

var GroceryList = React.createClass({
  handleClick: function(i) {
    console.log('You clicked: ' + this.props.items[i]);
  },

  render: function() {
    this.props.items.map(function(item, i) {
      return (
        <div
             onClick={this.handleClick.bind(this, i)}
             key={i}>
          {item}
        </div>
        );
    }, this)

    return ( <div>
               {this.props.items.map(function(item, i) {
                  return (
                    <div
                         onClick={this.handleClick.bind(this, i)}
                         key={i}>
                      {item}
                    </div>
                    );
                }, this)}
               <div>
                 {this.state.allSaved ? <div>
                                          all Saved!
                                        </div> : null}
               </div>
             </div>
      );
  }
});

React.render(
  <GroceryList items={['Apple', 'Banana', 'Cranberry']} />, mountNode
);

JSXExpressionContainers can contain valid javascript, code that so far was just hidden from the main esformatter, even when it can actually format thosed blocks, in order to be more predictable in the kind of formatting done by the esformatter-jsx.

The main issue here is that in order to be able to format them I have to recursively call again esformatter.format I was worried about the performance, but it actually worked pretty well.

So this works, but in order to update the demo, I should be able to browserify it, and since the esformatter module is just a dev dependency and not a dependency, and I don't want to hardcode the dependency on esformatter-jsx which can lead to weird results, then the bundle fails.

So i was thinking that you can pass to plugins the main esformatter object when the setOptions method is called. That would be nice that way I don't have to require the module from within the plugin and just use the same instance, and be able to browserify the file and update the demo.

Regards

millermedeiros commented 9 years ago

the idea of not passing the esformatter object was because I did not want plugins to rely on the internal methods from esformatter.. but now that API is pretty stable I think we can pass it to plugins.

maybe a second argument on plugin.setOptions() is good enough.


in case you're curious, we don't pass the esformatter intentionally, first plugin implementation was passing the utils module to all the plugins (utils was basically extracted as rocambole-node, rocambole-token, rocambole-linebreak, rocambole-whitespace)...

royriojas commented 9 years ago

Thanks @millermedeiros

Hope this can be fixed, if you need help with this one I guess is simple enough so I can help you if you want.