Webpack's removes classnames when minifying/uglifying ES6 code with inheritance
Asked Answered
T

2

8

Webpack's removes classnames when minifying/uglifying ES6 code with inheritance:

There's MVCE code which we try to minify/uglify:

Class Child:

const ParentClass = require('parent');

class Child extends ParentClass{
    constructor(){
        super();
    }
}

module.exports = Child;

index.js which invokes Child class:

const Child = require('./classes_so/child');

let child = new Child();

console.log(child.constructor.name);

Module Parent inside node_modules:

class Parent {
    constructor() {
        if (this.constructor.name === 'Parent'){
            throw new TypeError("Parent class is abstract - cant be instance");
        }
    }

}

module.exports = Parent;

The whole output I'll post to the end of the question, here I want to post only relevant lines which I think cause a wrong behavior (lines 33-37 from original output):

n.exports = class extends r {
        constructor() {
            super();
        }
    };

Why a classname is missing here: class extends r? I expect that value will be mangled but will exist, can I consider it as a bug? I tried to use keep_classnames flag but it keeps original class names which unacceptable.

We're using:

  • Webpack: 3.11.0 (tried with 4, same behavior)
  • uglifyjs-webpack-plugin: 1.2.4 (tried with different plugins)
  • NodeJS: v6.9.1 and v8.9.1 (same output)
  • Complete project which demonstrate the issue: webpack-uglify-inheritence

Update 1:

Our webpack.config.js:

const webpack = require('webpack');
const UglifyJsPlugin = require('uglifyjs-webpack-plugin');
const path = require('path');
const fs = require('fs');

const nodeModules = {};
const localDependencies = ['.bin'];
fs.readdirSync('node_modules')
    .filter(function (x) {
        return localDependencies.indexOf(x) === -1;
    })
    .forEach(function (mod) {
        nodeModules[mod] = 'commonjs ' + mod;
    });

try {


    module.exports = {
        target: 'node',
        node: {
            console: false,
            global: false,
            process: false,
            Buffer: false,
            __filename: true,
            __dirname: true
        },

        entry: './index_so.js',

        output: {
            path: path.join(__dirname, 'build'),
            filename: 'index.js'
        },

        externals: nodeModules,
        plugins: [
            new webpack.IgnorePlugin(/\.(css|less)$/),
            new webpack.BannerPlugin({
                banner: 'require("source-map-support").install();',
                raw: true,
                entryOnly: false
            })
        ],
        devtool: 'sourcemap',

        module: {
            loaders: [
                {test: /\.json$/, loader: "json-loader"}
            ]
        },

        plugins: [
            new UglifyJsPlugin({
                uglifyOptions: {
                    compress: {
                        warnings: false
                    },
                    keep_classnames: false,
                    mangle: true,
                    output: {
                        beautify: true
                    }
                }
            })
        ]
    };
}
catch (e) {
    console.error(e);
}

The whole minified/uglified code from the example above:

!function(n) {
    var t = {};
    function e(r) {
        if (t[r]) return t[r].exports;
        var o = t[r] = {
            i: r,
            l: !1,
            exports: {}
        };
        return n[r].call(o.exports, o, o.exports, e), o.l = !0, o.exports;
    }
    e.m = n, e.c = t, e.d = function(n, t, r) {
        e.o(n, t) || Object.defineProperty(n, t, {
            configurable: !1,
            enumerable: !0,
            get: r
        });
    }, e.n = function(n) {
        var t = n && n.__esModule ? function() {
            return n.default;
        } : function() {
            return n;
        };
        return e.d(t, "a", t), t;
    }, e.o = function(n, t) {
        return Object.prototype.hasOwnProperty.call(n, t);
    }, e.p = "", e(e.s = 0);
}([ function(n, t, e) {
    let r = new (e(1))();
    console.log(r.constructor.name);
}, function(n, t, e) {
    const r = e(2);
    n.exports = class extends r {
        constructor() {
            super();
        }
    };
}, function(n, t) {
    n.exports = require("parent");
} ]);
Torto answered 1/4, 2018 at 12:54 Comment(8)
n.exports = class extends r { is a correct syntax. Do you get any error when you run your code, because of the missing class name?Brachylogy
I mean why are you ok that the class name is mangled, but it is not ok if it is missing? If you don't care about the name why do you care about its existence?Brachylogy
Yes, I get an error. We rely on a code this.constructor.name. If a code isn't minified it works otherwise it returns or an empty string (when the code runs in NodeJS v6.9.1) or a name of the class from which it was inherited in our case, it's Parent class what is definitely wrong. The last one happens when we run minified code on NodeJS v8.9.1Torto
I've created a simple project which demonstrate an issue, you can download it here: github.com/anatoly314/webpack-uglify-inheritenceTorto
But why do you want to realy on this.constructor.name === 'Parent' in a context where names can be mangled? I don't see that there is any bug with the minification, the bug is in my opinion in your code, and you should write somethign like if (this.constructor == Parent){Brachylogy
Our node_modules don't mangled, so if somebody will use it, it will contain the whole name. In an example a Parent inside node_modules so it isn't mangled.Torto
btw, maintainer of uglifyjs-webpack-plugin suggest that it's likely a bug, why do think that it's correct syntax?Torto
The syntax is valid, because the standard defined it as valid. Even if the error occurs due to the minification, the reason for the error is not this. The reason is that you use code that seems to work at first glance, but should not be used like this in general and especially not if the code or the code that uses it can be minified.Brachylogy
B
4

The problem in the given setup is not in the code of webpack or uglify but in this part of the code:

class Parent {
  constructor() {
    if (this.constructor.name === 'Parent') {
      throw new TypeError("Parent class is abstract - cant be instance");
    }
  }

}

module.exports = Parent;

The this.constructor.name === 'Parent' relays on the the class/function name, to test if Parent was directly instanced.

Instead of relaying one the name which can result in various problems, it would be a better idea to test if the constructor equals the class.

class Parent {
  constructor() {
    if (this.constructor === Parent) {
      throw new TypeError("Parent class is abstract - cant be instance");
    }
  }

}

module.exports = Parent;
Brachylogy answered 2/4, 2018 at 11:54 Comment(3)
It doesn't work in mangled version. this.constructor returns Parent in both case, when object was instantiated from Parent or Client.Torto
The response depends on a version of NodeJS, in v6.9.1 it returns an empty classname on the extended constructor and Parent if object instantiated from abstract object. In v8.9.1 it returns Parent when object instantiated from inherited or abstract class both.Torto
@Torto I tested it with 6.9.1 and newer and 8.9.1, and this.constructor === Parent returns false if new Child and true if new ParentClass. Are you sure that you correclty updated and relaoded your webpack-parent-class.Brachylogy
G
0

Try my lib typescript-class-helpers

import { CLASS } from 'typescript-class-helpers/browser';

@CLASS.NAME('Parent')
class Parent {
    constructor() {
        if (CLASS.getNameFromObject(child) === 'Parent'){
            throw new TypeError("Parent class is abstract - cant be instance");
        }
    }

}

@CLASS.NAME('Child')
class Child extends ParentClass{
    constructor(){
        super();
    }
}

let child = new Child();

console.log(CLASS.getNameFromObject(child)); // Child

With this you can minify your classes names and everything will be ok.

Gensmer answered 9/5, 2019 at 12:35 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.