Proposal to add bs-stdlib #2171

jchavarri · 2017-10-23T21:49:45Z

100% wip. Related to #2127.

Still many things missing, but hopefully serves as a base for discussion.

The idea is to include a new npm package in lib that gets automatically published every time the main package bs-platform gets published. This is an attempt to implement a poor man's version of the fixed / locked mode in Lerna 😁

jchavarri · 2017-10-23T21:58:08Z

jscomp/core/js_packages_info.ml

@@ -238,7 +238,9 @@ let string_of_module_id
          else  
            begin match module_system with 
              | AmdJS | NodeJS | Es6 -> 
-                dep_package_name // dep_path // js_file
+                (* HACK - TODO Replace bs-stdlib upstream and not here *)
+                let upd_dep_package_name = if dep_package_name = "bs-platform" then "bs-stdlib" else dep_package_name in


@bobzhang I tried to come up with a better solution than this, happy to take any pointers to where or how this replacement should be done.

jchavarri · 2018-03-15T22:59:02Z

@chenglou @bobzhang I was taking a look at this PR to see if there is anything recoverable.

If we apply this same approach to Belt, then there would be a belt package in npm, and it would be used everywhere. Like for the templates (example).

The tricky part comes when it comes to what will be included in this belt package. From @chenglou 's comment I understand it's only Set/Map and their variations.

So then, we'd need a way to conditionally change the printer to generate

var Belt_MapInt = require("belt/belt_MapInt.js");

instead of the current:

var Belt_MapInt = require("bs-platform/lib/js/belt_MapInt.js");

Whenever a function that belongs to a module published with belt is referenced.

Does that sound right to you? If yes, could you point me to the part of the code handles this? Is it js_dump_import_export? That seems too late already... I also saw string_of_module_id in js_name_of_module_id.ml that looks more promising.

chenglou · 2018-03-16T00:12:20Z

My naive thought was to just snatch lib/js or lib/es6 and publish it. Would that not work?

TheSpyder · 2018-03-16T09:24:38Z

you'd also need to change the package name that is require'd, it's currently hard coded to bs-platform.

My theory is bs-platform can list this stdlib as a dependency instead of shipping it built-in, and then publishing JS libraries is as simple as adding it to the dependency list explicitly.

chenglou · 2018-03-16T09:25:49Z

Ah!
A regex pass after it should be good enough for now right?

TheSpyder · 2018-03-16T09:28:40Z

well that's where this PR uses a string match to rename the require statements.

The difference between my theory and this PR is it puts the dependency in the template; I figure that's not necessary if bs-platform depends on it, unless you're publishing the JS to npm (and thus don't want to depend on the full bs-platform).

jchavarri · 2018-03-16T10:36:01Z

I am going to try and change the paths of the import / require statements before falling back to the redex. Changing this line in js_packages_info might do the trick.

@TheSpyder Right, if we include belt as dependency of bs-platform we don't need to change the templates at all. By default bs-platform would include belt, and at any point users just need to add belt to dependencies. If we pin the version of belt to the one in bs-platform it should be a very straight forward process.

jchavarri · 2018-03-18T22:51:48Z

@bobzhang @chenglou this is ready for a review. The PR does the following things:

Adds a new folder belt-js that contains all the generated JS code that users might need. This folder contains a package.json for a new package belt-js. Right now it contains both Belt and the stdlib, as well as every platform: es6, js and amdjs.
Adds a prepublish script that runs before publishing bs-platform. It checks that bs-platform and belt-js have the same version, and publishes belt-js if that's the case
Changes the path generation in js_packages_info.ml to generate belt-js/ paths instead of bs-platform paths
Changes all the package.json in the templates to include belt-js in dependencies.

If this PR goes forward, we'll have to make sure that when users update the version of bs-platform to the next one, they are aware that they need to run npm install --save belt-js in existing projects.

Side note: once belt-js is available as an independent package, I wonder if having bs-platform as a devDependency in the user projects makes sense... at the end of the day, most of the projects have that dependency linked to the global module.

TheSpyder · 2018-03-19T00:25:20Z

I think making templates include both platform and stdlib leads to unnecessary overhead if the project doesn't need the separation - the calls for this are still fairly infrequent. I suspect merging this would also break existing projects until they add belt-js?

How about:

Make bs-platform depend on the same belt-js version as itself, then it will update when bs-platform does and maintain compatibility with existing projects
Have some documentation about publishing to NPM that indicates belt-js can be added as an explicit dependency to avoid depending on bs-platform

TheSpyder · 2018-03-19T00:28:27Z

jscomp/core/js_packages_info.ml

@@ -238,7 +238,9 @@ let string_of_module_id
          else  
            begin match module_system with 
              | AmdJS | NodeJS | Es6 -> 
-                dep_package_name // dep_path // js_file
+                (* HACK - TODO Replace belt-js upstream and not here *)
+                let upd_dep_package_name = if dep_package_name = "bs-platform" then "belt-js" else dep_package_name in


This really does seem like the wrong place to make the swap, and it also breaks -global module system users (of which I am one)

Happy to move it somewhere else, if you have suggestions. I'll try to fix the -global platform.

I wish I could help, but I sadly haven't looked at the BuckleScript code much at all.

I also thought about this package_name in bs_version, that gets generated from build_version. @bobzhang would that be better? I'm unsure if that'd break other things.

jchavarri · 2018-03-19T01:08:56Z

Make bs-platform depend on the same belt-js version as itself, then it will update when bs-platform does and maintain compatibility with existing projects

@TheSpyder Good idea, I updated the PR to do that, so existing projects don't break. Removed the deps from templates too.

jchavarri · 2018-03-19T01:09:29Z

Hah the build fails now of course, as belt-js is not in npm 😅

TheSpyder · 2018-03-19T01:19:15Z

hmm, yeah now the build depends on belt-js which is a bit of a circular dependency. That's going to complicate things 🤔

jchavarri · 2018-03-19T02:25:02Z

I think it's fine, it's just this first time. In the future, when there would be a new version, calling npm publish would trigger the publication of belt-js first, so when the changes are pushed to GitHub, the CI would already have the latest version available.

In any case, the tests are still running against /lib/js so there shouldn't be any problems while developing new, backwards incompatible features. /belt-js should be just a "shallow" folder that only gets its contents before publishing a new version.

bobzhang · 2018-03-19T12:31:07Z

making belt consumable to js devs is indeed the goal we want to achieve, but we still need polish it a bit (to make it stable enough to allow somewhat API backwards compatibility guarantee ).
What's your current use case？

jchavarri · 2018-03-20T00:21:51Z

I would like to use it to replace some immutable libs in JS. I also wanted to help the folks that use Node-In-Reason, who need a simple way to deploy the stdlib as a dependency without having to manually extricate it from the bs-platform package.

I was talking to @chenglou about having belt-js initially in a separate repo, to be able to polish and iterate on the JS interface quicker.

TheSpyder · 2018-03-20T00:26:40Z

The idea is I want to:

Compile my project with BuckleScript (or check the .bs.js files into source control)
Publish to NPM
Not have the resulting package depend on bs-platform due to install size and/or time to compile ocaml concerns (shipping binaries will help with the speed, but even with the right ocaml switch my bs-platform is 161 megabytes). I want it to be a devDependency only.

This has little to do with the stability of belt, more that the BuckleScript compiled JS depends on files that currently only ship with bs-platform (pervasives, caml_*, curry etc etc). Extracting them will let us distribute packages that can be used inside JS projects much more easily; right now the need to have bs-platform makes them really only suitable for other BuckleScript projects (in which case why publish the JS output at all).

Personally this is the main thing holding me back from trying to get more reasonml into my big JS codebase. It's split across multiple packages, which is ideal for replacing small components, but the number of times bs-platform would then need to be installed makes it a bit prohibitive.

jchavarri · 2018-03-20T00:34:23Z

@TheSpyder makes a good point. I think there is a conflation between belt-js, which is still in early stages and needs refinement as @bobzhang mentions, and the publication of an independent package (let's say bs-platform-lib).

Probably both issues will require different solutions, as they have nothing to do with each other. Belt will require a lot of effort to polish the JS story (docs etc), while the second case will have to include other packages that are not Belt (stdlib, curry and such).

We could keep this PR about bs-platform-lib, while we move the work and discussion about Belt in JS to a different place.

TheSpyder · 2018-03-20T01:02:15Z

This PR publishes the entire lib folder, which means it'll include both belt and the stdlib. I would be very surprised if the stdlib isn't active in compiled code even once Belt is stable, at least for the foreseeable future.

bobzhang · 2018-03-20T01:33:42Z

How do you publish your nodejs to production server?

This is the workflow I imagined: you do npm install and bsb -make-world on your dev server. When publish, you have a script to prune those ocaml binaries and gzip your current directory to AWS(or your internal server)

I assume you don't do npm install on production server for security reasons, so the above workflow should resolve your concern?

TheSpyder · 2018-03-20T03:12:06Z

Ah perhaps now we're hitting the crux of the issue. I'm talking libraries, not applications.

For production, yes, I bundle and publish the application without any dependencies. For development, though, my application has a large tree of library dependencies. If I add BuckleScript to one of the core libraries, even though I can publish the .bs.js files developing other libraries would still need bs-platform just to get the stdlib those files require - for example this library, which is used by every other library in the application (not all of them public).

https://www.npmjs.com/browse/depended/@ephox/katamari

That's a lot of copies of bs-platform, both on every developer machine and our CI server.

jchavarri · 2018-03-28T22:34:14Z

I think there are two different target audiences, and maybe the fact that i've been confusing both cases didn't help: 😅

A. Reason / OCaml devs that want to have a way of expressing a dependency on bs-stdlib without having to include the whole bs-platform (like the case @TheSpyder has mentioned above)
B. JavaScript developers that might be interested in using the JS published version of Belt as a standalone package.

What this PR is trying to solve is actually A, so I'll change the PR name back to the original. @bobzhang If you think this is something interesting and potentially mergeable, I can keep working on this PR.

Option B (a polished JavaScript version of Belt) will be happening, but definitely not in this PR.

TheSpyder · 2018-03-29T03:38:46Z

Is option B actually something we want? I thought Belt would only really be useful to BuckleScript-compiled code, not developers working in pure JS. The module interfaces seem very ocaml-specific.

chenglou · 2018-03-29T03:56:25Z

Option B is a very explicitly but intuitionally under-promoted goal. This exposes a very usable library for pure JS users:

This way they can adopt the data structures of BS/Reason without needing to switch their whole language. Then value proposition of converting to BS/Reason thus increases because the cost of conversion would have been drastically lowered.

TheSpyder · 2018-03-29T04:00:12Z

Oh interesting! Yeah I can see that being beneficial. It's not like people need to know how the structures in Immutable.js work either :)

ncthbrt · 2018-04-20T09:13:31Z

@bobzhang: We're using bucklescript on Node for our backend. Issue with doing pruning is that it doesn't really work for docker containers. When running rm on a docker container, it doesn't actually reduce the size of the resultant image, in fact it gets bigger, as the new layer has to describe which files are to be removed. This has led us to try and do a bunch of hacks involving intermediate containers, but it is quite ugly, messy and increases build times due to the resultant increase in the number of layers.

TheSpyder · 2018-07-05T02:02:07Z

I'm starting to see more people talk about this and there are other related issues popping up (#2772, #2418). See this twitter thread:

https://twitter.com/_rrdelaney/status/1013936446133030912

seenaburns · 2018-07-31T15:05:28Z

Just throwing into the mix, I really appreciate the work being done here on this issue. Including bs-platform in my electron app pulls with it all sorts of developer tooling into the build, where I only need the few runtime *.js files, dramatically affecting startup times.

I hacked around this by extracting the bs-platform/lib/js/*.js files into a local module, and replacing the require references of the generated *.bs.js and all Reason dependencies in seenaburns/isolate/issues/19 but this is pretty flaky and it'd be significantly more helpful to have the compiled *.bs.js and community libraries depend on the minimal bs-stdlib discussed here.

bobzhang · 2019-05-23T06:17:10Z

@jchavarri

The idea is to include a new npm package in lib that gets automatically published every time the main package bs-platform gets published.

It is easy to publish the same version for the compiler and stdlib, how do you make sure that it is the same when people consume it?

jchavarri · 2019-05-23T07:24:56Z

@bobzhang Not sure, but probably the most reasonable path is the compiler having a peerDependency on the stdlib package.

For example, bs-platform (or bs-compiler if it's renamed) in v6.0.0 would have in its package.json:

  "peerDependencies": {
    "bs-stdlib": "^6.0.0"
  }

That way, any package management tool will warn if users are missing the right version of stdlib for the version of the compiler they're using.

The other way around (bs-stdlib having a peer dependency on bs-compiler) wouldn't work, because bs-stdlib will always be a dependency while bs-compiler will be a devDependency.

ELLIOTTCABLE · 2019-05-23T07:27:01Z

Simplest is to just declare a peerDependency in the compiler. Each release of the compiler peerDepends on a very specific version of the runtime. User devDeoendency’s the compiler, and direct-depends on the runtime; npm warns (as, presumably, the compiler) the user if the runtime-peer is missing. Worst failure-mode there is if the user installs the runtime as a devDep just like the compiler, instead of a runtime dep; can be mitigated by documentation and package.json-linting during compilation by bsb … Meanwhile, *runtime* checks are one option, overhead wouldn’t be much (monotonic integer version, perhaps?) … but I don’t like adding to the compiler output, personally.

On Thu, May 23, 2019 at 01:17 Hongbo Zhang ***@***.***> wrote: @jchavarri <https://github.com/jchavarri> The idea is to include a new npm package in lib that gets automatically published every time the main package bs-platform gets published. It is easy to publish the same version for the compiler and stdlib, how do you make sure that it is the same when people consume it? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2171>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAABSF7EPNAIWNERGUX2GTPWYZGZANCNFSM4EAOF3GQ> .

-- ⁓ ELLIOTTCABLE — fly safe. http://ell.io/tt

ELLIOTTCABLE · 2019-05-23T07:32:05Z

Hah! Beat to it by one minute. Real-world example: I follow exactly this pattern with my ppx ports; for instance, “ppx-deriving” peerDepends on the runtime “bs-deriving” package; and all this is then documented in the install-instructions for the package.

On Thu, May 23, 2019 at 02:26 Elliott Cable ***@***.***> wrote: Simplest is to just declare a peerDependency in the compiler. Each release of the compiler peerDepends on a very specific version of the runtime. User devDeoendency’s the compiler, and direct-depends on the runtime; npm warns (as, presumably, the compiler) the user if the runtime-peer is missing. Worst failure-mode there is if the user installs the runtime as a devDep just like the compiler, instead of a runtime dep; can be mitigated by documentation and package.json-linting during compilation by bsb … Meanwhile, *runtime* checks are one option, overhead wouldn’t be much (monotonic integer version, perhaps?) … but I don’t like adding to the compiler output, personally. On Thu, May 23, 2019 at 01:17 Hongbo Zhang ***@***.***> wrote: > @jchavarri <https://github.com/jchavarri> > > The idea is to include a new npm package in lib that gets automatically > published every time the main package bs-platform gets published. > > It is easy to publish the same version for the compiler and stdlib, how > do you make sure that it is the same when people consume it? > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub > <#2171>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AAAABSF7EPNAIWNERGUX2GTPWYZGZANCNFSM4EAOF3GQ> > . > -- ⁓ ELLIOTTCABLE — fly safe. http://ell.io/tt

-- ⁓ ELLIOTTCABLE — fly safe. http://ell.io/tt

bobzhang · 2021-02-02T08:10:04Z

For people who are interested, this is landed in master: #2772 (comment)

jchavarri changed the title ~~Proposal to add a hypothetical bs-stdlib~~ Proposal to add bs-stdlib Oct 23, 2017

jchavarri mentioned this pull request Oct 23, 2017

[Proposal] Publish /lib/js/* as an npm package #2127

Closed

jchavarri commented Oct 23, 2017

View reviewed changes

jchavarri mentioned this pull request Mar 9, 2018

[Belt] Small roadmap #2603

Closed

6 tasks

jchavarri added 4 commits March 18, 2018 22:16

Add bs-stdlib package

09a2953

Create new folder belt-js

f8d5169

Add belt-js to every template

f8abc79

Finish prepublish.sh

899114f

jchavarri changed the title ~~Proposal to add bs-stdlib~~ Publish belt-js as a separate package Mar 18, 2018

TheSpyder reviewed Mar 19, 2018

View reviewed changes

Add belt-js as dependency of bs-platform. Remove deps from templates

bd0e6b8

jchavarri changed the title ~~Publish belt-js as a separate package~~ Proposal to add bs-stdlib Mar 28, 2018

jchavarri mentioned this pull request Apr 9, 2018

The bs-platform package size is too large #2723

Closed

seenaburns mentioned this pull request Jul 31, 2018

Find more stable way of including bs-platform/lib/js/* seenaburns/isolate#19

Open

jchavarri mentioned this pull request Nov 16, 2018

Create, Export and Publish typed (or untyped) libraries. rescript-association/genType#95

Closed

bobzhang mentioned this pull request May 23, 2019

split off compiler as dev dependencies #3559

Closed

jchavarri closed this Sep 23, 2020

jchavarri deleted the proposal-bs-stdlib-package branch September 23, 2020 17:24

Proposal to add bs-stdlib #2171

Proposal to add bs-stdlib #2171

Conversation

jchavarri commented Oct 23, 2017 • edited Loading

jchavarri Oct 23, 2017 • edited Loading

Choose a reason for hiding this comment

jchavarri commented Mar 15, 2018

chenglou commented Mar 16, 2018

TheSpyder commented Mar 16, 2018

chenglou commented Mar 16, 2018

TheSpyder commented Mar 16, 2018

jchavarri commented Mar 16, 2018 • edited Loading

jchavarri commented Mar 18, 2018

TheSpyder commented Mar 19, 2018

TheSpyder Mar 19, 2018

Choose a reason for hiding this comment

jchavarri Mar 19, 2018

Choose a reason for hiding this comment

TheSpyder Mar 19, 2018

Choose a reason for hiding this comment

jchavarri Mar 19, 2018

Choose a reason for hiding this comment

jchavarri commented Mar 19, 2018

jchavarri commented Mar 19, 2018 • edited Loading

TheSpyder commented Mar 19, 2018

jchavarri commented Mar 19, 2018 • edited Loading

bobzhang commented Mar 19, 2018

jchavarri commented Mar 20, 2018

TheSpyder commented Mar 20, 2018

jchavarri commented Mar 20, 2018 • edited Loading

TheSpyder commented Mar 20, 2018

bobzhang commented Mar 20, 2018

TheSpyder commented Mar 20, 2018 • edited Loading

jchavarri commented Mar 28, 2018

TheSpyder commented Mar 29, 2018

chenglou commented Mar 29, 2018

TheSpyder commented Mar 29, 2018

ncthbrt commented Apr 20, 2018 • edited Loading

TheSpyder commented Jul 5, 2018

seenaburns commented Jul 31, 2018

bobzhang commented May 23, 2019

jchavarri commented May 23, 2019 • edited Loading

ELLIOTTCABLE commented May 23, 2019 via email

ELLIOTTCABLE commented May 23, 2019 via email

bobzhang commented Feb 2, 2021

jchavarri commented Oct 23, 2017 •

edited

Loading

jchavarri Oct 23, 2017 •

edited

Loading

jchavarri commented Mar 16, 2018 •

edited

Loading

jchavarri commented Mar 19, 2018 •

edited

Loading

jchavarri commented Mar 19, 2018 •

edited

Loading

jchavarri commented Mar 20, 2018 •

edited

Loading

TheSpyder commented Mar 20, 2018 •

edited

Loading

ncthbrt commented Apr 20, 2018 •

edited

Loading

jchavarri commented May 23, 2019 •

edited

Loading