rescript-lang / rescript-compiler

The compiler for ReScript.
https://rescript-lang.org
Other
6.66k stars 442 forks source link

bsconfig.json {"in-source": false} overriden by settings in dependent package #2419

Closed porcuquine closed 1 year ago

porcuquine commented 6 years ago

Package A, bsconfig.json contains:

{
  "package-specs":  {
    "in-source": false
  }
  …
}

Package A package.json contains:

{
  "scripts": {
    "build": "bsb -make-world"
  }
  …
}

Package B includes Package A as a bs-dependency located using a file uri.

Package B package.json contains:

{
  "scripts": {
    "build": "bsb -make-world"
  },
  "dependencies": {
    "pkg-a": "file:../pkg-a
  }
  …
}

Package B bsconfig.json contains:

{
  "bs-dependencies": ["pkg-a"],
    "package-specs":  {
    "in-source": true
  }
  …
}

From /pkg-a/:

> npm run build

results in the expected out-of-source build. Files are generated in /pkg-a/lib/js/src/....js.

However -- from /pkg-b/, after

> npm run build

We find .js files in /pkg-a/src/....js. The reason Package A's source tree is modified is that the file:... dependency is represented as a symlink in /pkg-b/node_modules.

If we modify Package B's bsconfig.json:

{
  "package-specs":  {
    "in-source": false
  }
  …
}

then we see the originally expected behavior. That is, no .js files in /pkg-a/src -- only in /pkg-a/lib/js/src.

Therefore, it seems that the outer (true) value of package-specs.in-source from Package A is taking precedence over the more-specific false specified in Package A's bsconfig.json.

This seems wrong.

bobzhang commented 6 years ago

I did not remember why I made such choice, will come back later. @clkunzang it seems it does not do harm either? Also, can you create a mini-repo on github for me to verify it? Thanks

porcuquine commented 6 years ago

It does no harm as long as you want the same behavior of in-source for both projects. I had the impression that"in-source": true might be encouraged as the simplest, standard way to build for BuckleScript projects. However, I don't want .js files appearing in the source of a primarily OCaml library to be used by a primarily BuckleScript project.

So this issue only shows up if I want to use different conventions for different types of projects, while also configuring to optimize for convenience of local development. This might be a real use case, but I personally am fine with configuring everything to build out-of-source.

I can set up an example repo.

porcuquine commented 6 years ago

Here is a repo minimally demonstrating the issue: https://github.com/clkunzang/bucklescript-build-issue

The first commit sets up the situation: https://github.com/clkunzang/bucklescript-build-issue/commit/c4ad5f634e5dc87ebc57e97a533ef33b32c7ada6

The second commit builds in pkg-a: https://github.com/clkunzang/bucklescript-build-issue/commit/102f03fd79bcdd6b47dd3332e5fd03b915341c1f

The third commit builds in pkg-b: https://github.com/clkunzang/bucklescript-build-issue/commit/d2d08420160fcc862364efc5e4dac9c295c72c75

As you can see, new build files are added to pkg-a, including the unwanted in-source JavaScript file: pkg-a/src/demo.bs.js.

giraud commented 6 years ago

Same problem here. I have a library for which I don't want to use in-source true, but I can't because my project depends on it and it has a different in-source policy.

knowbody commented 6 years ago

we have the same issue in reroute-native https://github.com/callstack/reroute-native/issues/103 @bobzhang any chance explaining how to solve it for applications that use our library?

dvisztempacct commented 6 years ago

I believe this will exactly duplicate the issue:

donviszneki@Lilys-MBP ~$ git init foo
Initialized empty Git repository in /Users/donviszneki/foo/.git/
donviszneki@Lilys-MBP ~$ cd foo
donviszneki@Lilys-MBP ~/foo$ bsb -init .
Symlink bs-platform in /Users/donviszneki/foo/.
donviszneki@Lilys-MBP ~/foo$ sed -i -e 's,// add your bs-dependencies here,"my-dependency",' bsconfig.json
donviszneki@Lilys-MBP ~/foo$ grep -A2 bs-dependencies bsconfig.json
  "bs-dependencies": [
      "my-dependency"
  ],
donviszneki@Lilys-MBP ~/foo$ mkdir node_modules/my-dependency
donviszneki@Lilys-MBP ~/foo$ cd node_modules/my-dependency
donviszneki@Lilys-MBP ~/foo/node_modules/my-dependency$ bsb -init .
Symlink bs-platform in /Users/donviszneki/foo/node_modules/my-dependency/.
donviszneki@Lilys-MBP ~/foo/node_modules/my-dependency$ sed -i -e 's/"in-source": true/"in-source": false/' bsconfig.json
donviszneki@Lilys-MBP ~/foo/node_modules/my-dependency$ grep in-source bsconfig.json
    "in-source": false
donviszneki@Lilys-MBP ~/foo/node_modules/my-dependency$ cat > src/demo.ml
let foo () = Js.log "HI"
donviszneki@Lilys-MBP ~/foo/node_modules/my-dependency$ bsb -make-world
ninja: Entering directory `lib/bs'
[2/2] Building src/demo.mlast.d
[1/1] Building src/demo.cmj
donviszneki@Lilys-MBP ~/foo/node_modules/my-dependency$ find . -name \*.bs.js
./lib/js/src/demo.bs.js
donviszneki@Lilys-MBP ~/foo/node_modules/my-dependency$ rm -fr lib
donviszneki@Lilys-MBP ~/foo/node_modules/my-dependency$ cd ../..
donviszneki@Lilys-MBP ~/foo$ cat > src/demo.ml
My_dependency.foo();
donviszneki@Lilys-MBP ~/foo$ mv -v node_modules/my-dependency/src/{demo,My_dependency}.ml
node_modules/my-dependency/src/demo.ml -> node_modules/my-dependency/src/My_dependency.ml
donviszneki@Lilys-MBP ~/foo$ bsb -make-world
Duplicated package: bs-platform /Users/donviszneki/foo/node_modules/bs-platform (chosen) vs /Users/donviszneki/foo/node_modules/my-dependency/node_modules/bs-platform in /Users/donviszneki/foo/node_modules/my-dependency
[2/2] Building src/My_dependency.mlast.d
[1/1] Building src/My_dependency.cmj
ninja: Entering directory `lib/bs'
[2/2] Building src/demo.mlast.d
[1/1] Building src/demo.cmj
donviszneki@Lilys-MBP ~/foo$ find node_modules/my-dependency -name \*.bs.js
node_modules/my-dependency/src/My_dependency.bs.js
ELLIOTTCABLE commented 4 years ago

As a tertiary element of this issue: our project needs not only the sub-packages in-source setting maintained, but the entire package-specs section — that is, there's two build-output-modes, both of which must be present for the dependant-project to build:

{
   // ...
   "package-specs": [
      {
         "module": "es6",
         "in-source": true
      },
      {
         "module": "commonjs",
         "in-source": false
      }
   ]
}

(For context, this is because both the CommonJS / ES5 and ESM / ES6 builds are shipped to npm; but the descendant project very specifically needs the ESM-build to be in the tree at build-time so a Rollup bundling-process can analyze and tree-shake it.)

This would be less of an issue for us, if -clean-world didn't remove things that -build-world then won't re-create. For example, in our monorepo,

  1. Lerna builds the dependency-project, invoking the build-process which runs, basically, cd ./bs-dependency && bsb -clean-world and cd ./bs-dependency && bsb -make-world therein. This works fine.
  2. Lerna starts the build of the consumer-project, which first involves cleaning the tree with a corresponding cd ./bs-consumer && bsb -clean-world
  3. and then starts the consumer's build-process, which now fails, because cd ./bs-consumer && bsb -make-world doesn't re-create the .bs.js files in ./bs-dependency that were destroyed by -clean-world.

At the moment, we can work around this by changing the cleaning process in dependent-projects to use bsb -clean instead of bsb -clean-world, but that's a less-than-ideal solution, for obvious reasons. )=

Hope this information is helpful! Keep up the good work; I know the team of BuckleScript maintainers is way smaller than it should be — remember take a break when you need to, and de-stress! <3

(I'd open a new Issue for this, except this -clean-world behaviour doesn't really need to change, as long as -build-world will recursively build the full projects; generators, package-specs, and all! (= )

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.