bisect_ppx is not marked as a dev dependency

[amsross]

bisect_ppx is a regular dependcy in bsconfig.json, so depending code will not build until it has been been installed.

➜  foo npm run build

> foo@0.1.0 clean /foo
> bsb -clean-world

File "bsconfig.json", line 1
Error: package bisect_ppx not found or built
- Did you install it?
- If you did, did you run `bsb -make-world`?

[mlms13]

I noticed that even if you put it in"bs-dev-dependencies", the fact that it's listed in "ppx-flags" means that Bucklescript still tries to build it in downstream projects. I think Bucklescript probably needs the concept of "dev-ppx-flags", which doesn't currently exist.

[Risto-Stevcev]

It needs to be part of bs-dependencies according to the bisect_ppx readme, that's also the case with the native side of things but I'm using buckleescript to generate the code coverage.
I'll close this one unless there's something I'm missing

[mlms13]

If it needs to be part of bs-dependencies, it should probably also be part of dependencies in the package.json. Currently, if you npm install bs-bastet in a project, it won't build unless that project also does npm install bisect_ppx.

I should mention, I asked about this in the #bucklescript channel, but I'd still be interested in hearing @aantron's feedback.

[amsross]

If it needs to be part of bs-dependencies, it should probably also be part of dependencies in the package.json.

This would be my suggestion.

I'll submit a PR in about 10-12 hours (my AM).

[aantron]

See rescript-lang/rescript#3761, also rescript-lang/rescript#3716 (comment). For now, the easiest thing to do is indeed to include bisect_ppx in dependencies in package.json, but I hope that the need for this can be eliminated soon. Note that instrumentation still won't be triggered unless BISECT_ENABLE is set to yes, so the PPX will run, but it will be a no-op. It is, however, annoying to have the PPX installing for downstream users. cc @bobzhang

The current alternative to listing bisect_ppx in dependencies is to remove bisect_ppx from bs-dependencies and ppx-flags before each release, but that is tedious and error-prone.

[aantron]

Also cc @TheSpyder for suggesting a dev-only bsconfig.json :)

[Risto-Stevcev]

Ah my bad, I'll merge in the PR once it comes in