unassert-js / unassert

Encourages programming with assertions by providing tools to compile them away.
MIT License
192 stars 13 forks source link

Won't work for sub functions #18

Open falsandtru opened 3 years ago

falsandtru commented 3 years ago

Now the sub functions such as assert.doesNotThrow won't be removed. Probably this is a bug on the upstream.

falsandtru commented 3 years ago

@twada Is this working on your projects?

falsandtru commented 3 years ago

I found this is caused by the update of espurify.

twada commented 3 years ago

@falsandtru Oh thank you for reporting. I'm going to investigate this issue.

falsandtru commented 3 years ago

Thanks.

twada commented 3 years ago

@falsandtru I cannot reproduce the case since assert.doesNotThrow still removed properly. Would you make a small reproduction case?

https://github.com/unassert-js/unassert/pull/19

falsandtru commented 3 years ago

Did you test after clean install?

twada commented 3 years ago

yes

$ mkdir pd-unassert
$ cd pd-unassert
$ npm init -y
Wrote to /Users/takuto/work/pd-unassert/package.json:

{
  "name": "pd-unassert",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "keywords": [],
  "author": "",
  "license": "ISC"
}

$ npm install --save-dev unassert-cli
npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN pd-unassert@1.0.0 No description
npm WARN pd-unassert@1.0.0 No repository field.

+ unassert-cli@1.0.0
added 43 packages from 130 contributors and audited 43 packages in 3.71s

9 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

$ cat <<EOT >> prod.js
heredoc> const assert = require('assert');
heredoc> function fail() {
heredoc>   throw new Error('fail');
heredoc> }
heredoc> assert.doesNotThrow(function () {
heredoc>   fail();
heredoc> });
heredoc> EOT
$ cat prod.js
const assert = require('assert');
function fail() {
  throw new Error('fail');
}
assert.doesNotThrow(function () {
  fail();
});
$ npx unassert prod.js
function fail() {
    throw new Error('fail');
}
$ 
falsandtru commented 3 years ago

Indeed. I couldn't make a simple repro. FYI, the leaked assertions are this: https://github.com/falsandtru/pjax-api/commit/265f135050411c4db110db8162485dddea85bf54. Since they are few (almost all assertions are removed), this bug is stateful. Can you release the updated version at first?

falsandtru commented 3 years ago

The current repro:

  1. Clone https://github.com/falsandtru/pjax-api
  2. Checkout 4bf593c1d2350ca332f2e943f03ec99b8d4046bf
  3. Run npm i
  4. Run gulp dist
  5. See dist/pjax-api.js
twada commented 3 years ago

@falsandtru Thank you for your repro case! I'm going to find the root cause and fix it.

falsandtru commented 3 years ago

Since the current espurify is broken, can you release v2.0.1 as the latest?

twada commented 3 years ago

No, no, espurify is not broken but unssert do. I'll cut some bugfix releases ASAP.

falsandtru commented 3 years ago

OK.

twada commented 3 years ago

Since the current espurify is broken, can you release v2.0.1 as the latest?

First I've tagged espurify@2.0.1 as latest.

falsandtru commented 3 years ago

That's what I intended. Thanks.

twada commented 3 years ago

FYI: Released espurify 2.1.1 as bugfix