rescript-compiler icon indicating copy to clipboard operation
rescript-compiler copied to clipboard

fuse bsconfig into package.json

Open aspeddro opened this issue 3 years ago • 11 comments

This PR is a proposal to read dependencies, dev dependencies and name from package.json

aspeddro avatar Jun 21 '22 16:06 aspeddro

Need to resolve pinned dependencies. They are currently specified in bs-dependencies. Is workspaces in package.json equivalent?

aspeddro avatar Jun 21 '22 17:06 aspeddro

Need to resolve pinned dependencies. They are currently specified in bs-dependencies. Is workspaces in package.json equivalent?

See this repository (note the branch) for a minimal example of workspaces: https://github.com/Minnozz/rescript-reproduce-issue/tree/pinned-dependencies-issue

Minnozz avatar Jun 28 '22 17:06 Minnozz

Need to resolve pinned dependencies. They are currently specified in bs-dependencies. Is workspaces in package.json equivalent?

See this repository (note the branch) for a minimal example of workspaces: https://github.com/Minnozz/rescript-reproduce-issue/tree/pinned-dependencies-issue

thank you @Minnozz

An example of monorepo https://github.com/aspeddro/rescript-monorepo-experiments/tree/monorepo-test-take-one. The pinned-dependencies are defined in package.json

aspeddro avatar Jun 28 '22 22:06 aspeddro

Could you explain the proposal at high level? This is still draft, so maybe in flow. But seems to introduce a breaking change to existing projects, while one would expect purely additive functionality.

cristianoc avatar Jun 29 '22 14:06 cristianoc

Problem

  • Currently dependencies and dev dependencies are added to package.json and bsconfig.json
  • The name should be the same in bsconfig.json and package.json

This information is already in package.json.

Solution

Read package.json, iterate over the dependencies and devDependencies and check if a bsconfig.json exists.

This approach implies a loss of performance if the package.json is large, but I believe that people are willing to pay this cost in exchange for this ergonomics.

One caveat, I'm exploring the compiler, I might be breaking some rule.

Related: #5278

aspeddro avatar Jun 29 '22 19:06 aspeddro

But seems to introduce a breaking change to existing projects, while one would expect purely additive functionality.

If I keep pinned-dependencies in bsconfig.json will it break for existing projects?

aspeddro avatar Jun 29 '22 19:06 aspeddro

But seems to introduce a breaking change to existing projects, while one would expect purely additive functionality.

If I keep pinned-dependencies in bsconfig.json will it break for existing projects?

I guess the question is: is it true that any project correctly configured today will still work exactly the same after this change?

Also, I guess bsconfig allows to specify a subset of the dependencies defined in package.json. If so, is there still a way to do so. Should there be?

If one can do zero breaking changes, that's ideal. If some breaking changes are unavoidable, there's still the pending thing of using rescript.json, and one can consider to introduce the change at the same time as the rename.

cristianoc avatar Jun 29 '22 19:06 cristianoc

I guess the question is: is it true that any project correctly configured today will still work exactly the same after this change?

No, after this change package.json is required.

One option to maintain compatibility is to keep name in bsconfig.json.

aspeddro avatar Jun 29 '22 20:06 aspeddro

I guess the question is: is it true that any project correctly configured today will still work exactly the same after this change?

No, after this change package.json is required.

One option to maintain compatibility is to keep name in bsconfig.json.

Can you expand a bit on the explanation, even though it might seem obvious. And with a concrete example?

cristianoc avatar Jun 30 '22 00:06 cristianoc

Sorry for delay.

Here an valid example build_tests/react-ppx that will fail.

  • Compile search for package.json to read name field, but file is not found.

aspeddro avatar Jul 14 '22 19:07 aspeddro

Sorry for delay.

Here an valid example build_tests/react-ppx that will fail.

  • Compile search for package.json to read name field, but file is not found.

I suggest to turn this into a proposal with no breaking changes, give a description which is a bit more detailed than the one above (with examples) and try to reach consensus on the forum.

cristianoc avatar Jul 14 '22 23:07 cristianoc