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

Allow webpack stats option to overwrite default behaviour #261

Merged
merged 2 commits into from
Oct 26, 2017

Conversation

dangcuuson
Copy link
Contributor

@dangcuuson dangcuuson commented Oct 26, 2017

What did you implement:

Closes #260

How did you implement it:

When console log the compile stats, if webpack.config.js has stats configured, use it. Otherwise use default behaviour.

How can we verify it:

Using example/serverless-offline with default config:

image

After add stats: 'minimal' to webpack config:
image

Todos:

  • Write tests
  • Write documentation
  • Fix linting errors
  • Make sure code coverage hasn't dropped
  • Provide verification config / commands / resources
  • Enable "Allow edits from maintainers" for this PR
  • Update the messages below

Is this ready for review?: YES
Is it a breaking change?: NO

@HyperBrain
Copy link
Member

Thanks. The implementation looks good 🙌
Could you additionally add something to the README that mentions the possibility to override the stats with a proper webpack config (maybe only that it is supported)? Then poeple do not have to figure it out by asking questions.

@dangcuuson
Copy link
Contributor Author

@HyperBrain it's done. 😄

Copy link
Member

@HyperBrain HyperBrain left a comment

Choose a reason for hiding this comment

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

Now it's complete! Thank you.

@HyperBrain HyperBrain merged commit 0002a40 into serverless-heaven:master Oct 26, 2017
@dashmug
Copy link

dashmug commented Nov 14, 2017

This doesn't do anything on my project.

Here's my config:

'use strict'

const path = require('path')
const { CheckerPlugin } = require('awesome-typescript-loader')
const slsWebpack = require('serverless-webpack')
const webpack = require('webpack')

module.exports = {
    entry: slsWebpack.lib.entries,
    target: 'node',
    output: {
        libraryTarget: 'commonjs2',
        path: path.join(__dirname, 'build'),
        filename: '[name].js',
    },
    module: {
        loaders: [
            { test: /\.ts$/, loader: 'awesome-typescript-loader' },
            { test: /\.ejs$/, loader: 'ejs-loader' },
        ],
    },
    externals: ['bcrypt', 'aws-sdk', 'aws-xray-sdk'],
    resolve: {
        extensions: ['.ts', '.js'],
    },
    devtool: 'source-map',
    plugins: [
        new CheckerPlugin(),
        new webpack.DefinePlugin({
            'process.env.NODE_ENV': JSON.stringify('production'),
        }),
    ],
    stats: 'minimal',
}

Package versions:

    serverless: ^1.24.1
    serverless-webpack: ^4.0.0
    webpack: ^3.8.1

I still get the same display in the console even though I added stats: 'minimal'.

@HyperBrain
Copy link
Member

@dashmug That's strange.
@dangcuuson Any idea?

@dangcuuson
Copy link
Contributor Author

@HyperBrain not sure. I'll have a look after work.
@dashmug could you tell me which command you use to run your serverless? Did you use serverless-offline, or local invoke?

@dashmug
Copy link

dashmug commented Nov 14, 2017

I used sls webpack.

@dangcuuson
Copy link
Contributor Author

dangcuuson commented Nov 15, 2017

I found the problem:

If you have package option in your serverless.yml, it will map your webpack config into an array, which kind of break my implementation.

The fix should be easy.

@HyperBrain is there a good way to unit test this thing?

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.

3 participants