loader-runner icon indicating copy to clipboard operation
loader-runner copied to clipboard

refactoring to ES6?

Open tarang9211 opened this issue 9 years ago • 4 comments

Hey, looking to contribute to this! Does any of this codebase require refactoring to ES6?

tarang9211 avatar Jan 20 '17 20:01 tarang9211

Make sure you are using a good eslint editor (I use VSCODE and it is very effective). We use node-eslint-plugin so that we are not using es6 features not compatible with node 4.3. Any time you use let and const make sure you add "use strict" to top of file. And we do not process our library with any transpilers so extra babel features are not desirable.

  • [ ] Grab the new eslint file from webpack/webpack
  • [ ] Update eslint and mocha deps
  • [ ] Install node-eslint-plugin (dep should be listed in webpack/webpack)
  • [ ] If there are ES5 Classes, convert them to ES6
  • [ ] Convert .map calls to for let or for var statements (this is a perf increase) IE https://github.com/webpack/loader-runner/blob/master/lib/LoaderRunner.js#L256

Extra Credit:

  • [ ] Adding more tests
  • [ ] Adding benchmarking suite (to verify performance changes or regressions)

TheLarkInn avatar Jan 20 '17 21:01 TheLarkInn

Hi @tarang9211 I accidentally closed this PR https://github.com/webpack/loader-runner/pull/11 via me making the mistake of accidentally doing a git push to master after fixing your branch when I checked it out. Is there any way you could reopen it please? Thanks and sorry for the trouble.

TheLarkInn avatar Jan 22 '17 23:01 TheLarkInn

So remaining things to fix on the checklist: (based on your PR):

  • [ ] Add engines property that is equal to the one also found in webpack/webpack package.json
  • [ ] Add files property and add [/lib] in package.json

These should fix the errors you were seeing.

TheLarkInn avatar Jan 22 '17 23:01 TheLarkInn

@TheLarkInn no worries, I just reopened it.

tarang9211 avatar Jan 23 '17 07:01 tarang9211