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

Migrate: Format output config to use prettier #70

Closed
TheLarkInn opened this issue Feb 23, 2017 · 5 comments
Closed

Migrate: Format output config to use prettier #70

TheLarkInn opened this issue Feb 23, 2017 · 5 comments

Comments

@TheLarkInn
Copy link
Member

TheLarkInn commented Feb 23, 2017

Do you want to request a feature or report a bug?
Feature

What is the current behavior?
Poorly formatted configs, or new ast nodes that are generated appear inline and hard to read after --migrate.

If the current behavior is a bug, please provide the steps to reproduce.

What is the expected behavior?
Be prettier (put intended) than what is output now.

If this is a feature request, what is motivation or use case for changing the behavior?
I was able to add @jlongster's prettier and pass the string in to be formatted:

Below is how I implemented this feature (at a very rough level).

transformations/index.js

  return pretty.format(ast.toSource(recastOptions), {singleQuote: true, tabWidth: 4});

The results were, damn fine looking!

var path = require('path');
var webpack = require('webpack');

// lets take this webpack v1 config and migrate it to v2 !!! <3333

module.exports = {
    debug: true,
    devtool: 'eval',
    entry: ['./src/index'],
    output: {
        path: path.join(__dirname, 'dist'),
        filename: 'index.js'
    },
    plugins: [
        new webpack.OccurenceOrderPlugin(),
        new webpack.optimize.UglifyJsPlugin({
            sourceMap: true
        }),
        new webpack.optimize.LoaderOptionsPlugin({
            debug: true,
            minimize: true
        })
    ],
    resolve: {
        modules: ['foo']
    },
    module: {
        rules: [
            {
                test: /\.js$/,
                use: 'babel-loader',
                include: path.join(__dirname, 'src')
            }
        ]
    }
};

Thoughts @pksjce @ev1stensberg @kenwheeler @okonet? Spoke with folks at prettier and they said if passing a direct string is a concern and overhead we could look into the future passing a direct AST.

@TheLarkInn TheLarkInn changed the title Migrate: Format output config Migrate: Format output config to use prettier Feb 23, 2017
@TheLarkInn
Copy link
Member Author

TheLarkInn commented Feb 23, 2017

@Could technically be some low hanging fruit for someone to tackle also!!! Given we all agree we should format the ast before its applied to disk.

@okonet
Copy link
Contributor

okonet commented Feb 23, 2017

I agree we should do that. My only concern is that it still not be a fit for many devs and keeping in mind prettier has almost zero configuration options we won't be able to please everyone. Which is okay for me :)

@TheLarkInn
Copy link
Member Author

Yes, that was one of my concerns as well. I would assume that the most suspect ones though would be spaces and tabs which I think we could in the future accept args and then pass to prettier as args/opts

@pksjce
Copy link

pksjce commented Feb 27, 2017

I think prettier is a great option. We are anyway working around this issue. It would remove the need for https://github.com/webpack/webpack-cli/blob/master/package.json#L42 and thus enable us to use yarn on our cli :)

I will PR this.

@evenstensberg
Copy link
Member

Closed per #207

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

No branches or pull requests

4 participants