Skip to content

Make Array.prototype.includes polyfill not enumerable #1133

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

Closed
1 of 3 tasks
zwug opened this issue Oct 4, 2017 · 4 comments
Closed
1 of 3 tasks

Make Array.prototype.includes polyfill not enumerable #1133

zwug opened this issue Oct 4, 2017 · 4 comments

Comments

@zwug
Copy link
Contributor

zwug commented Oct 4, 2017

  • Operating System: macOS Sierra 10.12.6
  • Node Version: 4.8.4
  • NPM Version: 2.15.11
  • webpack Version: 2.7.0
  • webpack-dev-server Version: 2.9.1
  • This is a bug
  • This is a feature request
  • This is a modification request

My webpack build fails on stylus-loader, because stylus-loader uses for .. in loop to iterate through an array of files. And because includes property is enumerable, there is an iteration with it which causes failure. I have already created an issue in stylus-loader, but i think webpack-dev-server should change the polyfill property setting too. I can make a pull request with Object.defineProperty to fix this.

Code

...
{
  test: /\.styl$/,
  use: [
     { loader: 'style-loader', options: { sourceMap: true } },
     { loader: 'css-loader', options: { sourceMap: true } },
     { loader: 'postcss-loader', options: { sourceMap: true } },
     { loader: 'stylus-loader', options: { sourceMap: true } },
   ],
},
...
  // webpack.config.js

code in stylus-loader:

function normalizePaths(paths) {
  for(var i in paths) {
    paths[i] = path.normalize(paths[i]);
  }
  return paths;
}

webpack-dev-server polyfill setting:

if (!Array.prototype.includes) {
  Array.prototype.includes = require('array-includes');
}

Expected Behavior

Array.prototype.includes should not be enumerable

Actual Behavior

Array.prototype.includes is enumerable

For Bugs; How can we reproduce the behaviour?

node v4.8.4 + stylus-loader v3.0.1 should do it

@shellscape
Copy link
Contributor

@zwug thanks for checking in. could you elaborate a bit on what you mean by "enumerable" in this case? I'm not 100% sure I understand how polyfills.js is conflicting with stylus.

@zwug
Copy link
Contributor Author

zwug commented Oct 4, 2017

Here is a sample snippet with the problem I am facing

if (!Array.prototype.someFunc) {
  Array.prototype.someFunc = function() {
    return 'something';
  };
}

const items = ['potato', 'bread', 'milk'];

for (let index in items) {
  console.log(items[index]);
}

// potato
// bread
// milk
/*
ƒ () {
    return 'something';
  }
*/

Stylus loader is using the same for..in loop to iterate through an array of strings. And it fails trying to treat function as a string. By enumerable I mean the descriptor value. This code works correctly:

if (!Array.prototype.someFunc) {
  Object.defineProperty(Array.prototype, 'someFunc', {
      enumerable: false,
      configurable: false,
      writable: false,
      value: function() {
        return 'something';
      }
  });
}

const items = ['potato', 'bread', 'milk'];

for (let index in items) {
  console.log(items[index]);
}

// potato
// bread
// milk

@shellscape
Copy link
Contributor

Thanks for elaborating. If you'd like to contribute the change for this so you get credit, we'd be happy to accept. Otherwise I'm good to make this change as well.

@zwug
Copy link
Contributor Author

zwug commented Oct 4, 2017

I'll do it

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

2 participants