rescript-lang / rescript

ReScript is a robustly typed language that compiles to efficient and human-readable JavaScript.
https://rescript-lang.org
Other
6.76k stars 449 forks source link

Problem with yarn 2 & 3 and inlined externals #5317

Closed em closed 2 months ago

em commented 3 years ago

A little context - npm still hoists all packages to the root package's node_modules directory. Yarn used to do the same. This has always been a deduplication optimization, and never reflected the way node.js's module system actually worked. node.js just resolves dependencies with trial and error, walking up the filesystem and looking for node_modules directories.

It was always a quirk of these hoisting strategies that packages always got hoisted even when a dependency only existed as deep subdependency.

Say you have two rescript packages, let's call them A and B. Package B depends on package A.

package A:

Car.res
@module("car")
external drive = "drive"

package B:

A.Car.drive()

This generates the following:

in package A: car.bs.js:

// Generated by ReScript, PLEASE EDIT WITH CARE
/* This output is empty. Its source's type definitions, externals and/or unused code got optimized away. */

in package B:

import * as Car from "car"
Car.drive()

The import path "car" generated here, is based on the assumption that hoisting has taken place. Which is not guaranteed. Instead I would argue that when a separate rescript module is referenced in this way, the expected output should be:

import * as Car from "A/node_modules/car"

The current behavior does not work with Yarn 2/3 unless the "car" is independently added as a dependency of package B. But then you have to manually ensure the versions always match so the binding in package A are correct in package B.

Another solution is to write all bindings like so:

Car.res
@module("car")
external drive_ext = "drive"
let drive = drive_ext

This bypasses the optimization and resolutions get corrected.

bobzhang commented 2 years ago

Note the work around could be done in a slightly more elegant way:

@module("car")
external drive = "drive"
let drive = drive
TheSpyder commented 2 years ago

The current behavior does not work with Yarn 2/3 unless the "car" is independently added as a dependency of package B.

I have just always assumed transitive ReScript dependencies must be added as explicit dependencies (in both package.json and bsconfig.json). ReScript compiles projects in a flat style, it's not well suited to mapping nested dependency structures.

stale[bot] commented 8 months 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.

cknitt commented 2 months ago

Closing stale issue.