Skip to content
This repository was archived by the owner on May 1, 2020. It is now read-only.

Conversation

@elevente
Copy link

@elevente elevente commented Apr 3, 2017

Short description of what this resolves:

I added a command line flag(--noMinifyJs) to disable minifyJs on prod build, so we can use the uglifyJs plugin in webpack config and generate a sourcemap file which is correct. Issue: #856

Changes proposed in this pull request:

  • add --noMinifyJs command line flag

@DavidStrausz
Copy link

@elevente very nice :) I tested your PR combined with a custom webpack config (with uglifyjs plugin: new webpack.optimize.UglifyJsPlugin({ sourceMap: true })) and ionic_generate_source_map set to true. Works very well when doing a production build like this: ionic build android --prod --noMinifyJs. Sentry is now able to map production errors correctly.
Also interesting: The size of my main.js bundle increased by 35KB. Maybe the uglifyjs webpack plugin needs to be configured differently.

BR
David

@larssn
Copy link

larssn commented Apr 4, 2017

@DavidStrausz try {compress: true, sourceMap: true}

@DavidStrausz
Copy link

@larssn thanks but no difference -> compress is enabled by default according to the plugin docs.

@larssn
Copy link

larssn commented Apr 4, 2017

Ah, you're right

@larssn
Copy link

larssn commented Apr 4, 2017

I'm a bit quick on the trigger these days. Removed the code I posted, as it doesn't work.

Might as well dump the entire custom webpack config, as that does work. Maybe someone will get use from it.

var path = require('path');
var webpack = require('webpack');
var ionicWebpackFactory = require(process.env.IONIC_WEBPACK_FACTORY);
const UglifyJSPlugin = require('uglifyjs-webpack-plugin');

console.log('-------------- CUSTOM WEBPACK CONFIG --------------');

var plugins = [
    ionicWebpackFactory.getIonicEnvironmentPlugin()
];

if (process.env.IONIC_ENV === 'prod') {
    plugins.push(new UglifyJSPlugin({ compress: true, sourceMap: true }));
}

module.exports = {
    entry: process.env.IONIC_APP_ENTRY_POINT,
    output: {
        path: '{{BUILD}}',
        publicPath: 'build/',
        filename: process.env.IONIC_OUTPUT_JS_FILE_NAME,
        devtoolModuleFilenameTemplate: ionicWebpackFactory.getSourceMapperFunction(),
    },
    devtool: process.env.IONIC_SOURCE_MAP_TYPE,

    resolve: {
        extensions: ['.ts', '.js', '.json'],
        modules: [path.resolve('node_modules')]
    },

    module: {
        loaders: [
            {
                test: /\.json$/,
                loader: 'json-loader'
            },
            {
                test: /\.ts$/,
                loader: process.env.IONIC_WEBPACK_LOADER
            }
        ]
    },

    plugins: plugins,

    // Some libraries import Node modules but don't use them in the browser.
    // Tell Webpack to provide empty mocks for them so importing them works.
    node: {
        fs: 'empty',
        net: 'empty',
        tls: 'empty'
    }
};

@danbucholtz
Copy link
Contributor

Sorry, this is more overhead for us that at this time we don't wish to take on.

A prod build consists of:

  1. AoT (--aot)
  2. Optimizations (--optimizejs)
  3. minify JS (--minifyjs)
  4. minify CSS (--minifycss)

You can add those options to ionic build to do a custom prod build.

Thanks,
Dan

@larssn
Copy link

larssn commented May 14, 2017

@danbucholtz Going back to this. How would we get around not using --prod; because using the above doesn't set IONIC_ENV to prod.

May I suggest going forward, that we can somehow customise what exactly is part of a prod build? Something like:

prod: {
   aot: true,
   optimizejs: true,
   minifyjs: false,
   minifycss: true
}

@clarkmcnally
Copy link

clarkmcnally commented Dec 1, 2017

Is --noMinifyJs still included in the latest ionic/app-scripts? I don't see it in the source code

@clarkmcnally
Copy link

I also have noticed that my source maps in sentry are broken on 3.1.* but work on 3.0.1

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants