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

Not able to exclude from rollup bundle #1099

Closed
StoneCypher opened this issue May 6, 2019 · 9 comments
Closed

Not able to exclude from rollup bundle #1099

StoneCypher opened this issue May 6, 2019 · 9 comments

Comments

@StoneCypher
Copy link

StoneCypher commented May 6, 2019

Explanation

So, there's a couple other issues about this, and one of them seems to suggest this is solved already. I'm not sure if that means my situation is different, or if I used the fix incorrectly, or what.

It doesn't help that my understanding of the situation is shaky.

I am in the process of moving a library from an older build process using flow, babel, and browserify to a newer build process using typescript and rollup. For the most part, the results are much better.

I would like to run my existing test set, which performed as 100% coverage under my old setup, to get 100% coverage under my new setup.

Rollup now folds all contents into a single file, in a way that istanbul appears to be telling apart possibly through sourcemaps.

Problematically, one of the involved files is a generated parser from peg.js. Previously, I simply excluded it from my coverage reports, because getting coverage on a peg parser isn't particularly meaningful or practical. However, as I've moved to rollup, I seem to be unable to find a way to exclude the file.

Goal

I would like to exclude src/js/jssm-dot.js from my coverage results, so that they can be shipped to coveralls.

Struggle

I've tried:

  1. --exclude on the command line
  2. -x on the command line
  3. -n on the command line
  4. exclude: in a .nycrc
  5. excludes: in a .nycrc
  6. exclude: in package.json
  7. excludes: in package.json
  8. gating with include: in .nycrc
  9. Both boolean states of exclude-after-remap, which I believe should be true
  10. Both boolean states of all, which I believe should be false
  11. An istanbul.yml exclusion
  12. Just deleting the source file in question to see if the parser would give up
  13. A wide range of combinations of these

At all times, the generated parser remains in my coverage, ruining my beautiful 100%.

Research

Issue 1042 seems to suggest that patch 1010 was meant to fix this, with inclusion in 14.0.0, and seems to have for someone whose situation appears to be similar to my own. It looks like this is what pull 982 and pull 275 on istanbul support.

I wasn't able to understand whether pull 1007 was relevant.

I think issue 956 is something different, though I'm not really sure.

Issue 953 seems tangentially related but points to the same 1010 resolution as the others.

Config

I have upgraded to 14.1.0 (which is current as of writing - I was on 11 prior to this refab) and ava 1.4.1 (was on 0.22.0).

Repo for reference

You can see 100% coverage here and the drop to 77% here. That drop is entirely about being unable to exclude the 2 megabyte generated peg parser.

Mainline coveralls at 100%, branch coveralls at 77%.

Appeal

Can you help me? I think I'm missing something. ☹️

@StoneCypher
Copy link
Author

The branch that I want to land is AttemptTsAndRollup

@coreyfarrell
Copy link
Member

@StoneCypher if you could provide a reduced reproduction it would be helpful. Unfortunately I do not have time to actually test with large projects right now.

One thing I noticed immediately, you have "exclude-after-remap": "src/js/jssm-dot.js" in your .nycrc. This option expects a boolean, not a string. It is enabled by default I suggest you not specify this option at all. Also there is no excludes option so you can just remove that in favor of exclude. The istanbul.yml file has nothing to do with nyc.

dist/jssm.es6.js.map references ../src/js/jssm-dot.js. Could you try excluding that (prefixed with the ../)? Not suggesting this is a fix, just a troubleshooting step.

@StoneCypher
Copy link
Author

One thing I noticed immediately, you have "exclude-after-remap": "src/js/jssm-dot.js" in your .nycrc. This option expects a boolean, not a string

Sorry, yeah, I've had it on both booleans and undefined too. That's something I found in a ticket that was later corrected. Removed just now

.

dist/jssm.es6.js.map references ../src/js/jssm-dot.js. Could you try excluding that (prefixed with the ../)? Not suggesting this is a fix, just a troubleshooting step.

I don't know how to exclude something from just a sourcemap. I'd be happy to try if you can tell me what to look for?

.

if you could provide a reduced reproduction it would be helpful.

I'll try, but also, if you know a place that has this working already, maybe I can figure it out by staring at their thing

@coreyfarrell
Copy link
Member

I don't know how to exclude something from just a sourcemap. I'd be happy to try if you can tell me what to look for?

What I mean is set "exclude": ["../src/js/jssm-dot.js"] in your .nycrc then try running the test again.

@StoneCypher
Copy link
Author

uh. a ... a different piece of source (i don't care if it's covered or not, it's a one-liner) gets removed, but the desired removal does not occur.

i had not realized that the target directory needed to be ../.

without that exclude:

  √ general » Illegal machines no probable action exits of non-action

  986 tests passed [21:06:43]

------------------------------|----------|----------|----------|----------|-------------------|
File                          |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
------------------------------|----------|----------|----------|----------|-------------------|
All files                     |    79.64 |     68.2 |    95.76 |    82.83 |                   |
 node_modules/reduce-to-639-1 |      100 |      100 |      100 |      100 |                   |
  index.js                    |      100 |      100 |      100 |      100 |                   |
 src/js                       |    76.52 |    68.19 |    86.93 |    81.02 |                   |
  jssm-dot.js                 |    76.52 |    68.19 |    86.93 |    81.02 |... 67,12674,12675 |
 src/js/tests                 |      100 |      100 |      100 |      100 |                   |
  array_transitions.js        |      100 |      100 |      100 |      100 |                   |
  arrow unicode.js            |      100 |      100 |      100 |      100 |                   |
  arrow.js                    |      100 |      100 |      100 |      100 |                   |
  colors.js                   |      100 |      100 |      100 |      100 |                   |
  comment.js                  |      100 |      100 |      100 |      100 |                   |
  compile.js                  |      100 |      100 |      100 |      100 |                   |
  cycles.js                   |      100 |      100 |      100 |      100 |                   |
  forced transitions.js       |      100 |      100 |      100 |      100 |                   |
  general.js                  |      100 |      100 |      100 |      100 |                   |
  graph node lists.js         |      100 |      100 |      100 |      100 |                   |
  histo.js                    |      100 |      100 |      100 |      100 |                   |
  language.js                 |      100 |      100 |      100 |      100 |                   |
  layout.js                   |      100 |      100 |      100 |      100 |                   |
  machine_attributes.js       |      100 |      100 |      100 |      100 |                   |
  machine_name.js             |      100 |      100 |      100 |      100 |                   |
  named lists.js              |      100 |      100 |      100 |      100 |                   |
  nominated states.js         |      100 |      100 |      100 |      100 |                   |
  parse actions.js            |      100 |      100 |      100 |      100 |                   |
  parse.js                    |      100 |      100 |      100 |      100 |                   |
  r639.js                     |      100 |      100 |      100 |      100 |                   |
  sample_select.js            |      100 |      100 |      100 |      100 |                   |
  seq.js                      |      100 |      100 |      100 |      100 |                   |
  shapes.js                   |      100 |      100 |      100 |      100 |                   |
  sm_tag.js                   |      100 |      100 |      100 |      100 |                   |
  special characters.js       |      100 |      100 |      100 |      100 |                   |
  state_declaration.js        |      100 |      100 |      100 |      100 |                   |
  stop light.js               |      100 |      100 |      100 |      100 |                   |
  stripes.js                  |      100 |      100 |      100 |      100 |                   |
  weighted_histo_key.js       |      100 |      100 |      100 |      100 |                   |
  weighted_rand_select.js     |      100 |      100 |      100 |      100 |                   |
  weighted_sample_select.js   |      100 |      100 |      100 |      100 |                   |
------------------------------|----------|----------|----------|----------|-------------------|

> jssm@5.14.2 nyc-html C:\Users\john\projects\jssm

with it:

 √ general » [anonymous]
  √ general » Illegal machines no probable action exits of non-action

  986 tests passed [21:07:29]

----------------------------|----------|----------|----------|----------|-------------------|
File                        |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
----------------------------|----------|----------|----------|----------|-------------------|
All files                   |    79.63 |     68.2 |    95.76 |    82.83 |                   |
 js                         |    76.52 |    68.19 |    86.93 |    81.02 |                   |
  jssm-dot.js               |    76.52 |    68.19 |    86.93 |    81.02 |... 67,12674,12675 |
 js/tests                   |      100 |      100 |      100 |      100 |                   |
  array_transitions.js      |      100 |      100 |      100 |      100 |                   |
  arrow unicode.js          |      100 |      100 |      100 |      100 |                   |
  arrow.js                  |      100 |      100 |      100 |      100 |                   |
  colors.js                 |      100 |      100 |      100 |      100 |                   |
  comment.js                |      100 |      100 |      100 |      100 |                   |
  compile.js                |      100 |      100 |      100 |      100 |                   |
  cycles.js                 |      100 |      100 |      100 |      100 |                   |
  forced transitions.js     |      100 |      100 |      100 |      100 |                   |
  general.js                |      100 |      100 |      100 |      100 |                   |
  graph node lists.js       |      100 |      100 |      100 |      100 |                   |
  histo.js                  |      100 |      100 |      100 |      100 |                   |
  language.js               |      100 |      100 |      100 |      100 |                   |
  layout.js                 |      100 |      100 |      100 |      100 |                   |
  machine_attributes.js     |      100 |      100 |      100 |      100 |                   |
  machine_name.js           |      100 |      100 |      100 |      100 |                   |
  named lists.js            |      100 |      100 |      100 |      100 |                   |
  nominated states.js       |      100 |      100 |      100 |      100 |                   |
  parse actions.js          |      100 |      100 |      100 |      100 |                   |
  parse.js                  |      100 |      100 |      100 |      100 |                   |
  r639.js                   |      100 |      100 |      100 |      100 |                   |
  sample_select.js          |      100 |      100 |      100 |      100 |                   |
  seq.js                    |      100 |      100 |      100 |      100 |                   |
  shapes.js                 |      100 |      100 |      100 |      100 |                   |
  sm_tag.js                 |      100 |      100 |      100 |      100 |                   |
  special characters.js     |      100 |      100 |      100 |      100 |                   |
  state_declaration.js      |      100 |      100 |      100 |      100 |                   |
  stop light.js             |      100 |      100 |      100 |      100 |                   |
  stripes.js                |      100 |      100 |      100 |      100 |                   |
  weighted_histo_key.js     |      100 |      100 |      100 |      100 |                   |
  weighted_rand_select.js   |      100 |      100 |      100 |      100 |                   |
  weighted_sample_select.js |      100 |      100 |      100 |      100 |                   |
----------------------------|----------|----------|----------|----------|-------------------|

@StoneCypher
Copy link
Author

the difference being that node_modules/reduce-to-639-1 got removed

in some sense this makes sense: node_modules/reduce-to-639-1 is only used, and only in one place, by the file i'd like to remove reference to

@coreyfarrell
Copy link
Member

Maybe I misunderstood, looks like the bundles in dist/** only contain the generated code? If that's the case you probably want "exclude": ["dist/**"] - this would make your tests run faster too by not instrumenting the bundles in the first place.

@StoneCypher
Copy link
Author

you understand correctly.

generated code is in build/. dist/ contains some cherry-picked subset of the generated code as expected for distribution (minified, alternate bundle loaders, sourcemaps, &c.)

i will exclude build and dist when i get home from work. speed isn't really an issue (this is why i love ava) - i currently run about a thousand tests in about two seconds. that number will be going way, way up soon.

@StoneCypher
Copy link
Author

I have no idea what happened here.

I went back that night, tried it, made no change

Today I had a few minutes to kill at work while I waited on something, so I tried your instructions again.

It just worked? What's weirder, I can look in travis and see a thing where the .nycrc was in exactly the state it currently is, and wasn't filtering correctly

I assume that what this actually means is that one of my other flailing changes was breaking whatever parsed the config, and that that change wasn't checked in, and since I'm on a different machine, I'm not stuck with it

This works fine. I can no longer repro. I will reopen this if the problem returns.

I believe this is PEBKAC. Sorry. Thanks for your time

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

No branches or pull requests

2 participants