common-shakeify - npm Package Compare versions

Comparing version 1.1.1 to 1.1.2

CHANGELOG.md

@@ -7,2 +7,5 @@ # common-shakeify change log
 
+## 1.1.2
+* Fix certain cases of exports in sequential expressions resulting in invalid syntax. ([@tornqvist](https://github.com/tornqvist) in [#43](https://github.com/browserify/common-shakeify/pull/43))
+
 ## 1.1.1
@@ -9,0 +12,0 @@ * Fixes infinite loop in new side-effect feature. ([#40](https://github.com/browserify/common-shakeify/pull/40))

example/app.js

@@ -1,2 +0,2 @@
-var message = require('./xyz').getMessage()
+const message = require('./xyz').getMessage()
 console.log(message)

example/build.js

@@ -1,6 +0,6 @@
-var path = require('path')
-var browserify = require('browserify')
-var commonShake = require('../')
+const path = require('path')
+const browserify = require('browserify')
+const commonShake = require('../')
 
-var b = browserify({ entries: path.join(__dirname, 'app.js') })
+const b = browserify({ entries: path.join(__dirname, 'app.js') })
   .plugin(commonShake, {
@@ -12,7 +12,7 @@ verbose: true
 // Minify & save
-var fs = require('fs')
-var concat = require('concat-stream')
-var uglify = require('uglify-js')
+const fs = require('fs')
+const concat = require('concat-stream')
+const uglify = require('uglify-js')
 b.pipe(concat(function (source) {
-  var minified = uglify.minify(source.toString('utf8'), {
+  const minified = uglify.minify(source.toString('utf8'), {
     mangle: false,
@@ -19,0 +19,0 @@ compress: true

example/xyz.js

@@ -1,2 +0,2 @@
-var target = exports.target = 'world'
+let target = exports.target = 'world'
 exports.getMessage = function () {
@@ -3,0 +3,0 @@ return 'hello ' + target

index.js

 'use strict'
 const path = require('path')
 const Analyzer = require('@goto-bus-stop/common-shake').Analyzer
-const evaluateConst = require('@goto-bus-stop/common-shake').evaluateConst
 const transformAst = require('transform-ast')
@@ -60,3 +59,3 @@ const wrapComment = require('wrap-comment')
     b.pipeline.get('label').unshift(createStream(opts, {
-      getSideEffects(name) {
+      getSideEffects (name) {
         const file = aliases.get(name) || name
@@ -70,3 +69,3 @@ let pkg
         }
-        return pkg && pkg.sideEffects === false ? false : true
+        return !(pkg && pkg.sideEffects === false)
       }
@@ -232,3 +231,3 @@ }))
     if (node.type === 'AssignmentExpression') {
-      var prefix = commentify(`${node.left.getSource()} =`) + ' '
+      let prefix = commentify(`${node.left.getSource()} =`) + ' '
       // Anonymous function and class expressions are parsed as statements if they
@@ -241,22 +240,36 @@ // are the first thing in a statement, which can happen if the `exports.xyz`
       // `void 0,function(){},exports.b=function(){}`
-      var isPossiblyAmbiguousExpression = node.right.type === 'FunctionExpression' || node.right.type === 'ClassExpression'
-      if (isPossiblyAmbiguousExpression && node.parent.type === 'SequenceExpression' ||
+      // In the case of non-function/class expressions, we can void the whole thing
+      // eg: `exports.a={},exports.b=''`
+      // becomes: `void {},void ''`
+      const isFunction = node.right.type === 'FunctionExpression'
+      const isAssignment = node.right.type === 'AssignmentExpression'
+      const isArrowFunction = node.right.type === 'ArrowFunctionExpression'
+      const isVariableDeclarator = node.parent.parent.type === 'VariableDeclarator'
+      if (
+      // persist sequential variable declarations
+      // eg: `var a = (0, exports.a = function(){})`
+        (!isVariableDeclarator && node.parent.type === 'SequenceExpression') ||
           // without this, `exports.a = exports.b = xyz` eliminating exports.a becomes `void exports.b = xyz`
           // which is invalid.
-          node.right.type === 'AssignmentExpression' ||
+          isAssignment ||
           // Don't output a statement containing only `void () => {}`
-          node.right.type === 'ArrowFunctionExpression') {
+          isArrowFunction
+      ) {
         // ignore alias assignment expression `exports.a = exports.b = exports.c`
         // unless the last argument is noname function
-        var isAliasAssignment = node.right.type === 'AssignmentExpression' && node.right.left.type === 'MemberExpression' && node.right.left.object.name === 'exports'
-        var isFunction = isAliasAssignment && node.right.right.type === 'FunctionExpression'
-        var isClass = isAliasAssignment && node.right.right.type === 'ClassExpression'
-        if (!isAliasAssignment || isFunction || isClass) {
-          prefix += 'void 0, '
+        const isAliasAssignment = isAssignment && node.right.left.type === 'MemberExpression' && node.right.left.object.name === 'exports'
+        const isAliasFunction = isAliasAssignment && node.right.right.type === 'FunctionExpression'
+        const isAliasClass = isAliasAssignment && node.right.right.type === 'ClassExpression'
+        if (!isAliasAssignment || isAliasFunction || isAliasClass) {
+          prefix += 'void '
+          if (isAssignment || isArrowFunction || isFunction || isAliasFunction || isAliasClass) {
+            prefix += '0, '
+          }
         }
-      }
-      // Make sure we can't accidentally continue a previous statement.
-      // eg in `exports.a = [0]` the `[0]` could continue a previous statement if that
-      // did not have a semicolon. By putting `void ` in front we force a new statement.
-      else if (node.parent.type === 'ExpressionStatement') {
+      } else if (
+        // Make sure we can't accidentally continue a previous statement.
+        // eg in `exports.a = [0]` the `[0]` could continue a previous statement if that
+        // did not have a semicolon. By putting `void ` in front we force a new statement.
+        node.parent.type === 'ExpressionStatement'
+      ) {
         prefix += 'void '
@@ -263,0 +276,0 @@ }

package.json

 {
   "name": "common-shakeify",
-  "version": "1.1.1",
+  "version": "1.1.2",
   "description": "browserify tree shaking plugin using @indutny common-shake",
   "main": "index.js",
   "scripts": {
-    "test": "tape test/test.js"
+    "lint": "standard",
+    "tests-only": "tape test/test.js",
+    "test": "npm run lint && npm run tests-only"
   },
@@ -40,8 +42,15 @@ "repository": {
     "babelify": "^8.0.0",
-    "browserify": "^16.1.1",
-    "concat-stream": "^1.6.1",
+    "browserify": "^17.0.0",
+    "concat-stream": "^2.0.0",
     "net-browserify-stub": "0.0.1",
-    "tape": "^4.9.0",
+    "standard": "^17.0.0",
+    "tape": "^5.6.1",
     "uglify-js": "^3.3.15"
+  },
+  "standard": {
+    "ignore": [
+      "test/*/*.js",
+      "example/bundle.*"
+    ]
   }
 }

test/api.js

@@ -1,7 +0,7 @@
-var test = require('tape')
-var path = require('path')
-var browserify = require('browserify')
-var commonShakeify = require('../')
+const test = require('tape')
+const path = require('path')
+const browserify = require('browserify')
+const commonShakeify = require('../')
 
-var sampleEntry = path.join(__dirname, 'simple/app.js')
+const sampleEntry = path.join(__dirname, 'simple/app.js')
 
@@ -8,0 +8,0 @@ test('should work as a plugin', function (t) {

test/funny-exports/expected.js

@@ -16,2 +16,4 @@ (function(){function r(e,n,t){function o(i,f){if(!n[i]){if(!e[i]){var c="function"==typeof require&&require;if(!f&&c)return c(i,!0);if(u)return u(i,!0);var a=new Error("Cannot find module '"+i+"'");throw a.code="MODULE_NOT_FOUND",a}var p=n[i]={exports:{}};e[i][0].call(p.exports,function(r){var n=e[i][1][r];return o(n||r)},p,p.exports,r,e,n,t)}return n[i].exports}for(var u="function"==typeof require&&require,i=0;i<t.length;i++)o(t[i]);return o}return r})()({1:[function(require,module,exports){
 
+/* common-shake removed: exports.f = */ void {},/* common-shake removed: exports.g = */ void 'g',/* common-shake removed: exports.h = */ void 0, () => {},/* common-shake removed: exports.i = */ void 0, function () {}
+
 },{}]},{},[1]);

test/funny-exports/funny.js

@@ -9,1 +9,3 @@ exports.exports = function () {}
 exports.d = exports.e = null
+
+exports.f = {},exports.g = 'g',exports.h = () => {},exports.i = function () {}

test/test.js

@@ -1,19 +0,20 @@
-var test = require('tape')
-var assert = require('assert')
-var fs = require('fs')
-var path = require('path')
-var browserify = require('browserify')
-var concat = require('concat-stream')
-var commonShake = require('../')
+'use strict'
 
+const test = require('tape')
+const fs = require('fs')
+const path = require('path')
+const browserify = require('browserify')
+const concat = require('concat-stream')
+const commonShake = require('../')
+
 function runTest (t, name) {
   t.plan(1)
-  var basedir = path.join(__dirname, name)
-  var optionsPath = path.join(basedir, 'options.js')
-  var options = {}
+  const basedir = path.join(__dirname, name)
+  const optionsPath = path.join(basedir, 'options.js')
+  let options = {}
   try { options = require(optionsPath)(t) } catch (err) {}
-  var entry = path.join(basedir, 'app.js')
-  var expected = path.join(basedir, 'expected.js')
-  var actual = path.join(basedir, 'actual.js')
-  var bundle = browserify({ entries: entry })
+  const entry = path.join(basedir, 'app.js')
+  const expected = path.join(basedir, 'expected.js')
+  const actual = path.join(basedir, 'actual.js')
+  const bundle = browserify({ entries: entry })
     .plugin(commonShake, options)
@@ -77,3 +78,3 @@ .bundle()
 test('external', function (t) {
-  var b = browserify({
+  const b = browserify({
     entries: path.join(__dirname, 'external/app.js')
@@ -91,3 +92,3 @@ })
 test('source maps', function (t) {
-  var b = browserify({
+  const b = browserify({
     entries: path.join(__dirname, 'source-map/app.js'),
@@ -104,3 +105,3 @@ debug: true,
 
-  var bundle = b.bundle()
+  const bundle = b.bundle()
   bundle.on('error', t.fail)
@@ -124,3 +125,3 @@
 test('dash-r', function (t) {
-  var b = browserify({
+  const b = browserify({
     entries: path.join(__dirname, 'dash-r/app.js')
@@ -131,3 +132,3 @@ })
 
-  var bundle = b.bundle()
+  const bundle = b.bundle()
   bundle.on('error', t.fail)
@@ -150,3 +151,3 @@
 test('dash-r node_modules', function (t) {
-  var b = browserify({
+  const b = browserify({
     entries: path.join(__dirname, 'dash-r-node-modules/app.js')
@@ -157,3 +158,3 @@ })
 
-  var bundle = b.bundle()
+  const bundle = b.bundle()
   bundle.on('error', t.fail)
@@ -177,3 +178,3 @@
 test('dash-r node_modules with full paths', { skip: true }, function (t) {
-  var b = browserify({
+  const b = browserify({
     fullPaths: true,
@@ -185,3 +186,3 @@ entries: path.join(__dirname, 'dash-r-node-modules/app.js')
 
-  var bundle = b.bundle()
+  const bundle = b.bundle()
   bundle.on('error', t.fail)
@@ -188,0 +189,0 @@

New alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Fixed alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

48512

increased by4.24%

Number of package files

72

increased by1.41%

Lines of code

900

increased by1.69%

Worsened metrics

Dev dependency count

9

increased by12.5%

No dependency changes