Skip to content

Allowing compiled code delivery #2772

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
snird opened this issue Apr 26, 2018 · 14 comments
Closed

Allowing compiled code delivery #2772

snird opened this issue Apr 26, 2018 · 14 comments

Comments

@snird
Copy link

snird commented Apr 26, 2018

Hi, so this is not exactly a bug, but it is a use case that I think deserve some attention.

Bucklescript is a compile to JS compiler that adds its own JS files to the bundle in the end, unlike TypeScript or Flow that compile "cleanly", without additional dependencies.

Since TS for example do not add any more dependencies, it acts just as a compiler. Technically, one can compile his code locally and deliver the code without Typescript as a dependency.
bs-platform on the other hand have JS files that the code depends upon.
As I noted in this previous issue: #2769
It means that currently, you have to deliver the entire compiler with npm to use bucklescript.

But as package dependencies, this is a norm too. to expect local compilation on the target everywhere.
as noted for example in the bs-json package with @glennsl rightful comment: glennsl/bs-json#34

This basically makes the npm packages of the BS community dynamic by default. npm install is not enough, you must compile locally too to make the JS files available.

This blocks alot of BS use cases -
In environments you just can't install OCaml compiler for various reasons.
In environments it will be wasteful and expensive (Essentially blocking server-less use case altogether)
Installing compiler takes 5~ minutes on CI. Build process is now substantially slower.

@glennsl points on the bs-json issue linked above are valid too.

I could only think on one solution for this so far, if the community even agrees a solution is needed..
To enable the option when compiling, to have additional vendor files target, where the bs-platform dependencies, and external packages compiled code dependencies will go to.
That way you can compile locally only once, have all the dependencies pre-compiled and ready to run. Reliably, pre-known, and not in any way dependant on the local machine compiler.

@snird
Copy link
Author

snird commented Apr 26, 2018

A discussion in discord regarding this, for reference:

Khady - Today at 2:29 PM
I'm not a webpack expert. So my setup is simple. Compile everything locally. The main of the app is in app.js (or app.bs.js). Use default webpack config with app.js as main. Launch webpack. Deploy the file generated by webpack. Done
the__spyder - Today at 2:30 PM
I use rollup, not webpack, but same idea
bundle your bucklescript output into a single file, deploy that
Snir - Today at 2:31 PM
Ah I see, so basically it is a "proxy" to compiling locally. I thought of it too, but it adds unnecessary complexity. My suggestion it just to allow through bucklescript a target for vendor JS
I deliver this code to a server side nodejs application, so I don't generally need bundling. It doesn't really makes sense
It's a nice hack for now though
the__spyder - Today at 2:34 PM
I think that's the recommended way of doing it tbh. I bundle for browser, so it just seems normal to me
but I think your issues more or less duplicate https://github.com/BuckleScript/bucklescript/pull/2171 and related discussion
GitHub
Proposal to add bs-stdlib by jchavarri · Pull Request #2171 · Bu...
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 th...

Snir - Today at 2:37 PM
Bundling code is a burden, and for browsers it is a given, so the tooling in place are very well made. For nodejs environment, bundling and handling sourcemaps of already compiled code is an unnecessary burden, and one that is somewhat harder to handle than browsers environment, because of tooling...
e.g - logger of server side errors for node js, using sentry through the standard winston, doesn't know about sourcemaps.
So bundling code in nodejs will make the error logs that much harder to work with
I understand that Reason and BS is mainly used in browsers, as its flagship is ReasonReact. I knew I would pay these small prices when I started with it (: Just trying to make you see my point of view, that this issue might be very beneficial from folks working on the server side
the__spyder - Today at 2:45 PM
yeah, and I agree with wanting to not deploy bs-platform, which is why I've been asking for the stdlib to be distributed separately (that PR I linked, and a few other related issues in the system)
I need it in order to develop a library that will be used by other JS libs, as I progressively try to integrate reason across a multi-library project
Snir - Today at 2:53 PM
What I did in the meantime is horrific even for me.. I just forked bs-platform and bs-json, for the first removed the compiler install postscript, for bs-json locally compiled the files so it'll have the JS files available..
Maybe I'll hack some "bundler" in the weekend. What is needed for node JS is just to go through every require() and take this JS files, put it in a target vendors lib, and change the require() to reflect that. feel hackish but it might solve this for now.
Khady - Today at 2:54 PM
That's a bundler
the__spyder - Today at 2:55 PM
@Snir the branch in that PR that I linked above creates a bs-stdlib package, you could use that rather than forking it yourself
it points all generated JS require links at the stdlib package instead of bs-platform
Snir - Today at 2:56 PM
@the__spyder right, but it only solves it for bs-platform dependencies. In my case, bs-json is another dependency for example.
Khady - Today at 2:56 PM
Also I think many packages shouldn't have bs-plateform as a dependency but as a peer dependency(edited)
Snir - Today at 2:57 PM
@Khady right, it is a bundler, but it won't put everything in one file and scrumble them in a way that will require source maps
the__spyder - Today at 2:57 PM
@Snir but if you use that forked bs-platform, then compile the bs-json dependency with -make-world, it should all work
Khady - Today at 2:57 PM
How do you ship other dependencies?
Snir - Today at 2:58 PM
@the__spyder Oh. I see, I didn't realized this. Now it's actually sounds awesome. I'll take a deeper look
the__spyder - Today at 2:58 PM
bsb -clean-world -make-world will recompile dependent projects, even if they've installed their own compiled output via npm
handy when they only ship cjs and not es6, for example
Snir - Today at 2:59 PM
@Khady through npm. The issue is the OCaml compiler. I can't install it on every environment for various reasons.. also, it's slow to install
the__spyder - Today at 2:59 PM
you're literally having the same problem I do but for different reasons :smiley:
Khady - Today at 2:59 PM
@the__spyder only works if bs-json ships it's js output in npm
Which won't work very often as bs generates a minimal js
Khady - Today at 3:00 PM
And so an empty js if nothing depends on the lib
the__spyder - Today at 3:01 PM
bs-json doesn't seem to ship any JS
so either way make world will compile using the forked bs-platform
Khady - Today at 3:03 PM
But he will not compile on the target
the__spyder - Today at 3:03 PM
ah I get you
yeah bs-json can't be listed as a regular dependency for deployment
this is all a bit messy at the moment, I really think extracting the stdlib from bs-platform would encourage more people to publish their JS to npm
Snir - Today at 3:04 PM
I'm not sure.
Khady - Today at 3:04 PM
I don't think publishing the js is a good idea for libs that are not meant to be consumed by javascript and so have a js only API
Snir - Today at 3:04 PM
Because it will enforce a specific BS version
Khady - Today at 3:05 PM
Because of difference of generated js between bs versions
the__spyder - Today at 3:05 PM
hmm true
Snir - Today at 3:05 PM
right, people will still need to compile locally
the__spyder - Today at 3:05 PM
I've seen recommendations of checking the generated JS into git, though, so I think it's the intention
Snir - Today at 3:05 PM
just need a way to move this compiled JS to a local folder, and not linked in the npm package folder
Basically, having the BS packages compile locally in their npm packages violates the whole idea of npm lock and determinism
It's the first platform that have npm packages that are essentially dynamic
the__spyder - Today at 3:07 PM
yeah, it does seem geared towards bundled output atm
Snir - Today at 3:07 PM
their output should live in the local
Khady - Today at 3:07 PM
Yeah the OCaml way doesn't map properly with npm at all

@simontegg
Copy link

Yes. This is blocking me using reason/bucklescript. I don't have that much control over the CI environment (AWS codebuild) and I'd rather not install the Ocaml compiler on every build.

My preferred workflow would have the third-party reason in node_modules compile to some other build directory and commit the .bs.js files into version control.

I think this is related or a duplicate of #2418

@banacorn
Copy link

banacorn commented Nov 5, 2018

@Khady (sorry having to mention you here, assuming that I tagged the right person XD)
Would you be so kind to guide me to an example of the webpack config that you've addressed in the discord discussion. Because I can't seem to get my webpack to work :(

Khady - Today at 2:29 PM
I'm not a webpack expert. So my setup is simple. Compile everything locally. The main of the app is in app.js (or app.bs.js). Use default webpack config with app.js as main. Launch webpack. Deploy the file generated by webpack. Done

@Khady
Copy link
Contributor

Khady commented Nov 5, 2018

With webpack 3, my config looks like this:

const path = require('path');
const slsw = require('serverless-webpack');

module.exports = {
  entry: slsw.lib.entries,
  resolve: {
    extensions: [
      '.js',
    ]
  },
  output: {
    libraryTarget: 'commonjs',
    path: path.join(__dirname, '.webpack'),
    filename: '[name].js',
  },
  target: 'node',
  externals: {
    // Possible drivers for knex - we'll ignore them
    // mysql is commented because it is used by our app
    'sqlite3': 'sqlite3',
    'mariasql': 'mariasql',
    'mssql': 'mssql',
    // 'mysql': 'mysql',
    'mysql2': 'mysql2',
    'oracle': 'oracle',
    'strong-oracle': 'strong-oracle',
    'oracledb': 'oracledb',
    'pg': 'pg',
    'pg-query-stream': 'pg-query-stream'
  }
};

@Khady
Copy link
Contributor

Khady commented Nov 5, 2018

And I have a full example that is documented here:
https://tech.ahrefs.com/create-aws-lambda-using-reasonml-and-bucklescript-15a0820ff55b
https://github.com/ahrefs/bs-aws-lambda/tree/master/example

@banacorn
Copy link

banacorn commented Nov 5, 2018

Thanks!!

@bobzhang
Copy link
Member

Not sure if it is too late, we will start working on it

@snird
Copy link
Author

snird commented May 23, 2019

Well, I'm since moved to a new company and got to evaluate Reason and BuckleScript again. So this will really help my point.
It's never too late. Thank you @bobzhang !

@aaronshaf
Copy link

aaronshaf commented Aug 15, 2019

How is this coming along?

@bobzhang
Copy link
Member

bobzhang commented Aug 16, 2019 via email

@aaronshaf
Copy link

This would be great for deploying to environments like zeit/now

@aaronshaf
Copy link

Is this still planned? :)

@bobzhang
Copy link
Member

for people who subscribe this issue, we will make major progress on this issue, stay tuned

@bobzhang
Copy link
Member

bobzhang commented Feb 2, 2021

It is a long awaited feature request. Landed in master.
User can add a field in next release:

"external-stdlib" : "@rescript/std"

@bobzhang bobzhang closed this as completed Feb 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants