Skip to content
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

chore(deps): update #70

Merged
merged 1 commit into from
Feb 4, 2019
Merged

chore(deps): update #70

merged 1 commit into from
Feb 4, 2019

Conversation

alexander-akait
Copy link
Member

This PR contains a:

  • bugfix
  • new feature
  • code refactor
  • test update
  • typo fix
  • metadata update

Motivation / Use-Case

Update deps

Breaking Changes

No

Additional Info

No

@filipesilva
Copy link

@evilebottnawi unsure why the windows CI is failing. Just checked out the PR locally on my windows machine with node 10.10 and it is passing:

kamik@RED-X1C6 MINGW64 /d/sandbox/terser-webpack-plugin (master)
$ git pr 70
From https://github.com/webpack-contrib/terser-webpack-plugin
 * [new ref]         refs/pull/70/head -> pr70
Switched to branch 'pr70'

kamik@RED-X1C6 MINGW64 /d/sandbox/terser-webpack-plugin (pr70)
$ npm i

> [email protected] install D:\sandbox\terser-webpack-plugin\node_modules\husky
> node husky install

husky > setting up git hooks
husky > done
npm WARN prepublish-on-install As of npm@5, `prepublish` scripts are deprecated.
npm WARN prepublish-on-install Use `prepare` for build steps and `prepublishOnly` for upload-only.
npm WARN prepublish-on-install See the deprecation note in `npm help scripts` for more information.

> [email protected] prepublish D:\sandbox\terser-webpack-plugin
> npm run build


> [email protected] prebuild D:\sandbox\terser-webpack-plugin
> npm run clean


> [email protected] clean D:\sandbox\terser-webpack-plugin
> del-cli dist


> [email protected] build D:\sandbox\terser-webpack-plugin
> cross-env NODE_ENV=production babel src -d dist --ignore 'src/**/*.test.js' --copy-files

Successfully compiled 5 files with Babel.
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: [email protected] (node_modules\fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for [email protected]: wanted {"os":"darwin","arch":"any"} (current: {"os":"win32","arch":"x64"})

added 1237 packages from 570 contributors and audited 86799 packages in 27.392s
found 0 vulnerabilities


kamik@RED-X1C6 MINGW64 /d/sandbox/terser-webpack-plugin (pr70)
$ npm run ci:test

> [email protected] ci:test D:\sandbox\terser-webpack-plugin
> npm run test -- --runInBand


> [email protected] test D:\sandbox\terser-webpack-plugin
> jest "--runInBand"

 PASS  test/terserOptions-option.test.js
 PASS  test/cache-option.test.js
 PASS  test/extractComments-option.test.js
 PASS  test/minify-option.test.js
 PASS  test/TerserPlugin.test.js
 PASS  test/validation.test.js
 PASS  test/sourceMap-option.test.js
 PASS  test/test-option.test.js
 PASS  test/warningsFilter-option.test.js
 PASS  test/parallel-option.test.js
 PASS  test/include-option.test.js
 PASS  test/exclude-option.test.js
 PASS  test/parallel-option-failure.test.js
 PASS  test/worker.test.js
 PASS  test/supports-multicompiler.test.js
 PASS  test/chunkFilter-option.test.js
 PASS  test/cjs.test.js

Test Suites: 17 passed, 17 total
Tests:       99 passed, 99 total
Snapshots:   507 passed, 507 total
Time:        16.594s
Ran all test suites.

kamik@RED-X1C6 MINGW64 /d/sandbox/terser-webpack-plugin (pr70)
$ node -v
v10.10.0

@alexander-akait
Copy link
Member Author

alexander-akait commented Feb 4, 2019

@filipesilva yep, wip on this, I really do not like when this happens, i need to figure out why it happened

@filipesilva
Copy link

For the record, also tried with the same node version as used in that Appveyor test and it also passed. Puzzling :/

kamik@RED-X1C6 MINGW64 /d/sandbox/terser-webpack-plugin (pr70)
$ nvm use 6.16.0
Now using node v6.16.0 (64-bit)

kamik@RED-X1C6 MINGW64 /d/sandbox/terser-webpack-plugin (pr70)
$ npm run ci:test

> [email protected] ci:test D:\sandbox\terser-webpack-plugin
> npm run test -- --runInBand


> [email protected] test D:\sandbox\terser-webpack-plugin
> jest "--runInBand"

 PASS  test/terserOptions-option.test.js (6.125s)
 PASS  test/cache-option.test.js
 PASS  test/extractComments-option.test.js
 PASS  test/minify-option.test.js
 PASS  test/TerserPlugin.test.js
 PASS  test/validation.test.js
 PASS  test/sourceMap-option.test.js
 PASS  test/test-option.test.js
 PASS  test/warningsFilter-option.test.js
 PASS  test/parallel-option.test.js
 PASS  test/include-option.test.js
 PASS  test/exclude-option.test.js
 PASS  test/parallel-option-failure.test.js
 PASS  test/worker.test.js
 PASS  test/supports-multicompiler.test.js
 PASS  test/chunkFilter-option.test.js
 PASS  test/cjs.test.js

Test Suites: 17 passed, 17 total
Tests:       99 passed, 99 total
Snapshots:   507 passed, 507 total
Time:        23.085s
Ran all test suites.

kamik@RED-X1C6 MINGW64 /d/sandbox/terser-webpack-plugin (pr70)
$ node -v
v6.16.0

exports[`when applied with \`minify\` option matches snapshot for \`terser\` minifier and \`parallel\` is \`true\`: errors 1`] = `
Array [
"Error: main.js from Terser
SyntaxError: Unexpected string",
Copy link
Member Author

@alexander-akait alexander-akait Feb 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@filipesilva Problem here 😕

  1. No problem before
  2. No problem on windows
  3. Problem on linux

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't reproduce :/

Tried running it multiple times as well to see if it was a race condition or something.

If I can help let me know. Otherwise I worry that I'm just bothering you while you work.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@filipesilva thanks for helping, do not worry, i will ping you if can't solve problem (but it is unlikely) 😄

@codecov
Copy link

codecov bot commented Feb 4, 2019

Codecov Report

Merging #70 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #70   +/-   ##
=======================================
  Coverage   98.57%   98.57%           
=======================================
  Files           5        5           
  Lines         280      280           
  Branches      109      109           
=======================================
  Hits          276      276           
  Misses          4        4

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0593d7c...0d78c28. Read the comment docs.

@codecov
Copy link

codecov bot commented Feb 4, 2019

Codecov Report

Merging #70 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #70   +/-   ##
=======================================
  Coverage   98.57%   98.57%           
=======================================
  Files           5        5           
  Lines         280      280           
  Branches      109      109           
=======================================
  Hits          276      276           
  Misses          4        4

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0593d7c...7d07b88. Read the comment docs.

@filipesilva
Copy link

filipesilva commented Feb 4, 2019

@evilebottnawi a fix was merged in terser that conflics with #69. Details in terser/terser#254 (comment).

Edit: tests here are still green with terser 3.16.1 so I guess my interpretation was wrong.

@alexander-akait
Copy link
Member Author

alexander-akait commented Feb 4, 2019

@filipesilva yep, problem with serialize-javascript + babel + jest

Input:

{ file: 'main.js',
      input:
       '/******/ (function(modules) { // webpackBootstrap\n/******/ \t// The module cache\n/******/ \tvar installedModules = {};\n/******/\n/******/ \t// The require function\n/******/ \tfunction __webpack_require__(moduleId) {\n/******/\n/******/ \t\t// Check if module is in cache\n/******/ \t\tif(installedModules[moduleId]) {\n/******/ \t\t\treturn installedModules[moduleId].exports;\n/******/ \t\t}\n/******/ \t\t// Create a new module (and put it into the cache)\n/******/ \t\tvar module = installedModules[moduleId] = {\n/******/ \t\t\ti: moduleId,\n/******/ \t\t\tl: false,\n/******/ \t\t\texports: {}\n/******/ \t\t};\n/******/\n/******/ \t\t// Execute the module function\n/******/ \t\tmodules[moduleId].call(module.exports, module, module.exports, __webpack_require__);\n/******/\n/******/ \t\t// Flag the module as loaded\n/******/ \t\tmodule.l = true;\n/******/\n/******/ \t\t// Return the exports of the module\n/******/ \t\treturn module.exports;\n/******/ \t}\n/******/\n/******/\n/******/ \t// expose the modules object (__webpack_modules__)\n/******/ \t__webpack_require__.m = modules;\n/******/\n/******/ \t// expose the module cache\n/******/ \t__webpack_require__.c = installedModules;\n/******/\n/******/ \t// define getter function for harmony exports\n/******/ \t__webpack_require__.d = function(exports, name, getter) {\n/******/ \t\tif(!__webpack_require__.o(exports, name)) {\n/******/ \t\t\tObject.defineProperty(exports, name, { enumerable: true, get: getter });\n/******/ \t\t}\n/******/ \t};\n/******/\n/******/ \t// define __esModule on exports\n/******/ \t__webpack_require__.r = function(exports) {\n/******/ \t\tif(typeof Symbol !== \'undefined\' && Symbol.toStringTag) {\n/******/ \t\t\tObject.defineProperty(exports, Symbol.toStringTag, { value: \'Module\' });\n/******/ \t\t}\n/******/ \t\tObject.defineProperty(exports, \'__esModule\', { value: true });\n/******/ \t};\n/******/\n/******/ \t// create a fake namespace object\n/******/ \t// mode & 1: value is a module id, require it\n/******/ \t// mode & 2: merge all properties of value into the ns\n/******/ \t// mode & 4: return value when already ns object\n/******/ \t// mode & 8|1: behave like require\n/******/ \t__webpack_require__.t = function(value, mode) {\n/******/ \t\tif(mode & 1) value = __webpack_require__(value);\n/******/ \t\tif(mode & 8) return value;\n/******/ \t\tif((mode & 4) && typeof value === \'object\' && value && value.__esModule) return value;\n/******/ \t\tvar ns = Object.create(null);\n/******/ \t\t__webpack_require__.r(ns);\n/******/ \t\tObject.defineProperty(ns, \'default\', { enumerable: true, value: value });\n/******/ \t\tif(mode & 2 && typeof value != \'string\') for(var key in value) __webpack_require__.d(ns, key, function(key) { return value[key]; }.bind(null, key));\n/******/ \t\treturn ns;\n/******/ \t};\n/******/\n/******/ \t// getDefaultExport function for compatibility with non-harmony modules\n/******/ \t__webpack_require__.n = function(module) {\n/******/ \t\tvar getter = module && module.__esModule ?\n/******/ \t\t\tfunction getDefault() { return module[\'default\']; } :\n/******/ \t\t\tfunction getModuleExports() { return module; };\n/******/ \t\t__webpack_require__.d(getter, \'a\', getter);\n/******/ \t\treturn getter;\n/******/ \t};\n/******/\n/******/ \t// Object.prototype.hasOwnProperty.call\n/******/ \t__webpack_require__.o = function(object, property) { return Object.prototype.hasOwnProperty.call(object, property); };\n/******/\n/******/ \t// __webpack_public_path__\n/******/ \t__webpack_require__.p = "";\n/******/\n/******/\n/******/ \t// Load entry module and return exports\n/******/ \treturn __webpack_require__(__webpack_require__.s = 0);\n/******/ })\n/************************************************************************/\n/******/ ([\n/* 0 */\n/***/ (function(module, __webpack_exports__, __webpack_require__) {\n\n"use strict";\n__webpack_require__.r(__webpack_exports__);\nclass Point {\n  constructor(x, y) {\n    this.x = x;\n    this.y = y;\n  }\n\n  static distance(a, b) {\n    const dx = a.x - b.x;\n    const dy = a.y - b.y;\n\n    return Math.hypot(dx, dy);\n  }\n}\n\n/* harmony default export */ __webpack_exports__["default"] = (Point);\n\n\n/***/ })\n/******/ ]);',
      inputSourceMap: null,
      commentsFile: false,
      extractComments: false,
      terserOptions: { output: { comments: /^\**!|@preserve|@license|@cc_on/i } },
      minify: [Function: minify] }

Serilizer output:

{"file":"main.js","input":"\u002F******\u002F (function(modules) { \u002F\u002F webpackBootstrap\n\u002F******\u002F \t\u002F\u002F The module cache\n\u002F******\u002F \tvar installedModules = {};\n\u002F******\u002F\n\u002F******\u002F \t\u002F\u002F The require function\n\u002F******\u002F \tfunction __webpack_require__(moduleId) {\n\u002F******\u002F\n\u002F******\u002F \t\t\u002F\u002F Check if module is in cache\n\u002F******\u002F \t\tif(installedModules[moduleId]) {\n\u002F******\u002F \t\t\treturn installedModules[moduleId].exports;\n\u002F******\u002F \t\t}\n\u002F******\u002F \t\t\u002F\u002F Create a new module (and put it into the cache)\n\u002F******\u002F \t\tvar module = installedModules[moduleId] = {\n\u002F******\u002F \t\t\ti: moduleId,\n\u002F******\u002F \t\t\tl: false,\n\u002F******\u002F \t\t\texports: {}\n\u002F******\u002F \t\t};\n\u002F******\u002F\n\u002F******\u002F \t\t\u002F\u002F Execute the module function\n\u002F******\u002F \t\tmodules[moduleId].call(module.exports, module, module.exports, __webpack_require__);\n\u002F******\u002F\n\u002F******\u002F \t\t\u002F\u002F Flag the module as loaded\n\u002F******\u002F \t\tmodule.l = true;\n\u002F******\u002F\n\u002F******\u002F \t\t\u002F\u002F Return the exports of the module\n\u002F******\u002F \t\treturn module.exports;\n\u002F******\u002F \t}\n\u002F******\u002F\n\u002F******\u002F\n\u002F******\u002F \t\u002F\u002F expose the modules object (__webpack_modules__)\n\u002F******\u002F \t__webpack_require__.m = modules;\n\u002F******\u002F\n\u002F******\u002F \t\u002F\u002F expose the module cache\n\u002F******\u002F \t__webpack_require__.c = installedModules;\n\u002F******\u002F\n\u002F******\u002F \t\u002F\u002F define getter function for harmony exports\n\u002F******\u002F \t__webpack_require__.d = function(exports, name, getter) {\n\u002F******\u002F \t\tif(!__webpack_require__.o(exports, name)) {\n\u002F******\u002F \t\t\tObject.defineProperty(exports, name, { enumerable: true, get: getter });\n\u002F******\u002F \t\t}\n\u002F******\u002F \t};\n\u002F******\u002F\n\u002F******\u002F \t\u002F\u002F define __esModule on exports\n\u002F******\u002F \t__webpack_require__.r = function(exports) {\n\u002F******\u002F \t\tif(typeof Symbol !== 'undefined' && Symbol.toStringTag) {\n\u002F******\u002F \t\t\tObject.defineProperty(exports, Symbol.toStringTag, { value: 'Module' });\n\u002F******\u002F \t\t}\n\u002F******\u002F \t\tObject.defineProperty(exports, '__esModule', { value: true });\n\u002F******\u002F \t};\n\u002F******\u002F\n\u002F******\u002F \t\u002F\u002F create a fake namespace object\n\u002F******\u002F \t\u002F\u002F mode & 1: value is a module id, require it\n\u002F******\u002F \t\u002F\u002F mode & 2: merge all properties of value into the ns\n\u002F******\u002F \t\u002F\u002F mode & 4: return value when already ns object\n\u002F******\u002F \t\u002F\u002F mode & 8|1: behave like require\n\u002F******\u002F \t__webpack_require__.t = function(value, mode) {\n\u002F******\u002F \t\tif(mode & 1) value = __webpack_require__(value);\n\u002F******\u002F \t\tif(mode & 8) return value;\n\u002F******\u002F \t\tif((mode & 4) && typeof value === 'object' && value && value.__esModule) return value;\n\u002F******\u002F \t\tvar ns = Object.create(null);\n\u002F******\u002F \t\t__webpack_require__.r(ns);\n\u002F******\u002F \t\tObject.defineProperty(ns, 'default', { enumerable: true, value: value });\n\u002F******\u002F \t\tif(mode & 2 && typeof value != 'string') for(var key in value) __webpack_require__.d(ns, key, function(key) { return value[key]; }.bind(null, key));\n\u002F******\u002F \t\treturn ns;\n\u002F******\u002F \t};\n\u002F******\u002F\n\u002F******\u002F \t\u002F\u002F getDefaultExport function for compatibility with non-harmony modules\n\u002F******\u002F \t__webpack_require__.n = function(module) {\n\u002F******\u002F \t\tvar getter = module && module.__esModule ?\n\u002F******\u002F \t\t\tfunction getDefault() { return module['default']; } :\n\u002F******\u002F \t\t\tfunction getModuleExports() { return module; };\n\u002F******\u002F \t\t__webpack_require__.d(getter, 'a', getter);\n\u002F******\u002F \t\treturn getter;\n\u002F******\u002F \t};\n\u002F******\u002F\n\u002F******\u002F \t\u002F\u002F Object.prototype.hasOwnProperty.call\n\u002F******\u002F \t__webpack_require__.o = function(object, property) { return Object.prototype.hasOwnProperty.call(object, property); };\n\u002F******\u002F\n\u002F******\u002F \t\u002F\u002F __webpack_public_path__\n\u002F******\u002F \t__webpack_require__.p = \"\";\n\u002F******\u002F\n\u002F******\u002F\n\u002F******\u002F \t\u002F\u002F Load entry module and return exports\n\u002F******\u002F \treturn __webpack_require__(__webpack_require__.s = 0);\n\u002F******\u002F })\n\u002F************************************************************************\u002F\n\u002F******\u002F ([\n\u002F* 0 *\u002F\n\u002F***\u002F (function(module, __webpack_exports__, __webpack_require__) {\n\n\"use strict\";\n__webpack_require__.r(__webpack_exports__);\nclass Point {\n  constructor(x, y) {\n    this.x = x;\n    this.y = y;\n  }\n\n  static distance(a, b) {\n    const dx = a.x - b.x;\n    const dy = a.y - b.y;\n\n    return Math.hypot(dx, dy);\n  }\n}\n\n\u002F* harmony default export *\u002F __webpack_exports__[\"default\"] = (Point);\n\n\n\u002F***\u002F })\n\u002F******\u002F ]);","inputSourceMap":null,"commentsFile":false,"extractComments":false,"terserOptions":{"output":{"comments":/^\**!|@preserve|@license|@cc_on/i}},"minify":function('terser').minify(file, {
            mangle: {
              reserved: ['baz']
            }
          })}

See on minify.

require('terser') was rewritten on function('terser').

@alexander-akait
Copy link
Member Author

Rewrite test for CI, but need investigate what the hell is going on 😕

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants