3

I have a script that adds methods to an object in order to keep everything in one place cleanly. This works well with the unminified js but once it gets minified it breaks and I don't why. Does anyone have experience with this, that can guide me to a solution? Why does my script throw 'Cannot redefine property: i' Error only after minifying?

Strategy.js

const Strategy = {};

Object.defineProperty(Strategy, 'map', { value: new Map() });

Object.defineProperty(Strategy, 'registerStrategy', {
  value: function registerStrategy(fn) {
    if (fn.name) {
      this.map.set(fn.name, fn);
      return Object.defineProperty(Strategy, fn.name, { value: fn });
    }

    return false;
  },
});

export default Strategy;

strategy-1.js

import Strategy from '../strategy';

export function strategy1(param) {
    return param + ' this is strategy 1.';
}

Strategy.registerStrategy(strategy1);

strategy-2.js

import Strategy from '../strategy';

export function strategy2(param) {
    return param + ' this is strategy 2.';
}

Strategy.registerStrategy(strategy2);

webpack.config.js

import path from 'path';

const webpackConfig = {
    module: {
        rules: [
            {
                test: /\.js$/,
                use: [{
                        loader: 'babel-loader',
                        options: {
                            presets: ['env'],
                        },
                    }],
            },
        ],
    },
    mode: 'development',
    watch,
    devtool: 'source-map',
    entry: './src/js/main.js',
    output: {
        filename: 'app.js',
        path: path.resolve(__dirname, './dist/'),
    },
};

export default webpackConfig;

Update based on answer from loganfsmyth

Strategy.js

const Strategy = {};

Object.defineProperty(Strategy, 'map', { value: new Map() });

Object.defineProperty(Strategy, 'registerStrategy', {
  value: function registerStrategy(fnName, fn) {
    if (fnName) {
      this.map.set(fnName, fn);
      return Object.defineProperty(Strategy, fnName, {
        value: fn,
        writable: true,
        configurable: false,
      });
    }

    return false;
  },
});

Strategy.registerStrategies = (strats) => {
  Object.keys(strats).forEach((name) => {
    Strategy.registerStrategy(name, strats[name]);
  });
};

export default Strategy;
colecmc
  • 3,133
  • 2
  • 20
  • 33
  • I see there is an issue when I define the property. This way is works: `Strategy, fn.name, { value: fn, writable: true, configurable: false, }` but this way is broken: `Strategy, fn.name, { value: fn }` – colecmc Feb 15 '19 at 16:21
  • 1
    Minifiers generally change function names, so you'd only ever want to use `fn.name` for debugging purposes, not for primary behavior, so you'd want your interface to be `Strategy.registerStrategy('strategy2', strategy2);`. – loganfsmyth Feb 15 '19 at 22:45

1 Answers1

1

Generally relying on function names for anything other than debugging purposes is a bad idea. Minifiers and other tooling often rename variables and functions to achieve their goal of making code smaller. This leads to fn.name returning a different value, so if you've hardcoded that name in other places where a minifier might either not change it, or change it to something else, your code will break.

For a case like yours, I'd generally expect the function to be:

Strategy.registerStrategy('strategy2', strategy2);

and then you can potentially generalize it and take advantage of ES6 object shorthand syntax:

Strategy.registerStrategies({ strategy2 });

and have that function be

Strategy.registerStrategies = function(strats) {
  Object.keys(strats).forEach(name => {
    Strategy.registerStrategy(name, strats[name]);
  });
};
loganfsmyth
  • 156,129
  • 30
  • 331
  • 251
  • Thanks for your feedback. How do I call a function without a function name? – colecmc Feb 16 '19 at 05:55
  • @colecmc The core this is that you should be calling the function either via variable (`foo()`, or a property names (`obj.foo()`, `obj[key]()`, and the same should be true when registering strategies. Because your current code gets the name via `fn.name` it is not relying on either of those approaches, the minifier can't know to change those accesses to use the renamed function. So my proposal of `registerStrategies` essentially makes registration use property access, which a minifier won't change. – loganfsmyth Feb 16 '19 at 08:29
  • I feel like I'm missing something. I've updated my question based on your answer. Is this what you had in mind? – colecmc Feb 18 '19 at 20:30
  • Your updated code still uses `fn.name`. You'd need to change `registerStrategy` to take 2 arguments, with the first being the name string. – loganfsmyth Feb 18 '19 at 22:24
  • OK, that works man. Thank you! I can see my function names in the minified code now! Excellent – colecmc Feb 19 '19 at 03:40