reasonml-community / bs-express Goto Github PK

As of february chrome auto treats cookies with no samesite set as lax, but the library lacks a way to explicitly set it to "none".

I'll submit a PR momentarily to fix this, just filing an issue in case there is some process needed.

Update: Opening message reproduced here. But actual discussion is being moved to https://reasonml.chat/t/rfc-the-future-of-bs-express/447

I have held the view that express while widely popular is a many senses a poor design. Largely for two reasons:

1. Its mutable api makes it easy in larger applications to accidentally open up security holes or introduce unpredictable request flows (have experienced this in practise).
2. Express also makes it possible to leave requests hanging.

Bs-express corrects deficiency number 2, but doesn't help to correct 1. Right now this project is being actively maintained but is more or less in a holding pattern (for example it isn't really getting new features nor a nice set of documentation). This is largely because I'm not very motivated to improve a design which I consider to be based upon an unsound foundation.

I would like to propose that this project (or a successor to this project) be broken into two parts:

The first is a layer which allows users to describe their web application in a more declarative manner. This layer would probably take inspiration from frameworks like Freya, Suave, and Giraffe which are all written in F#.
This layer would be platform independent.

The second layer would be where all the platform specific/moving parts live. This layer would take the abstract description of the web server, "compile" and then "execute" it. This could use Node.js or if using native, be run using something like LWT.

This approach means that native and node ecosystems can cross pollinate and provide a stronger, more idiomatic approach to writing web servers.

Keen to hear thoughts...

app.get and app.set aren't being bound currently. Since get is overloaded to be used for actual endpoint handler binding, we'd probably need to come up with alternative names. getAppSetting or setAppSetting maybe?

I've noticed that there is an interface file for the bindings which is not very common. I just briefly looked at the implementation and interface files and I couldn't see any functions or modules hidden by the interface files, maybe it could be removed altogether and the documentation transferred to the implementation file?

If you prefer to keep the interface file, then it'd be nice to copy the external and @bs annotations to the interface file so the bindings don't generate extra js functions as advised in the docs (https://bucklescript.github.io/docs/en/intro-to-external#docsNav), would you have anything against that?

See #5 for context. Replace loose bindings of 'a with Js.Json.t where appropriate.

When using bs-express with es6 modules, I get the error:

TypeError: Express is not a function

If I create the following binding locally, I don't get the error:

[@bs.module "express"] external express: unit => 'a = "default";

It seems that express is set up to be used with a few different module patterns: https://github.com/expressjs/express/blob/431f65305eb78eba337356ec7637e4ee13d84195/lib/express.js#L28

I saw a similar change here https://github.com/apollographql/reason-apollo/pull/80/files and hit something suspiciously similar here rhysforyou/bs-enzyme#14.

I'm a bit worried that changing this will break the commonjs usage. I don't know enough about the interop between all the different module specs.

Can we add bs-socket.io to the example?

It is unclear to me how to make them work together.

• MacOS 10.15.2 (19C57)
• Node 12.13.1
• Curl 7.64.1

curl 7.64.1 (x86_64-apple-darwin19.0) libcurl/7.64.1 (SecureTransport) LibreSSL/2.8.3 zlib/1.2.11 nghttp2/1.39.2
Release-Date: 2019-03-27
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp smb smbs smtp smtps telnet tftp
Features: AsynchDNS GSS-API HTTP2 HTTPS-proxy IPv6 Kerberos Largefile libz MultiSSL NTLM NTLM_WB SPNEGO SSL UnixSockets

Trying to extend the json middleware to take fileType, which itself seems straight forward, but I wanted to add tests as well, and on master even before my changes the tests fail.

Most of the tests get back a # Netscape HTTP Cookie File where there is not one on the reference data, and this is the only change I can find between the reference.data contents and the output in test.data. I've included test.data as a txt below.

test.data.txt

https://expressjs.com/en/4x/api.html#app.param

Hello,

I'm from a JavaScript background and I m quite interested in Reason. I wanted to create my first Express server, and I'm facing some misunderstanding while reading the example codebase.

I'm stuck at the lines after : https://github.com/reasonml-community/bs-express/blob/master/example/Index.re#L389 :

let onListen = (port, e) =>
  switch e {
  | exception (Js.Exn.Error(e)) =>
    Js.log(e);
    Node.Process.exit(1)
  | _ => Js.log @@ "Listening at http://127.0.0.1:" ++ string_of_int(port)
  };

App.listen(app, ~onListen=onListen(3000), ());

From what I can read, onListen isn't asynchronous and the declared onListen function is called directly, like synchronously :

App.listen(app, ~onListen=onListen(3000), ());

In JavaScript (using the same syntax, omitting the "standard Expressjs APIs), we would have made something like :

App.listen(app, { onListen: onListen }, () => null); // or something like this, never mind :p

It's quite disturbing and I can't get the point out there. How does this work ?

And by the way, thank you for this awesome binding module ❤️

Hi,
is there any reason why Response doesn't have "render" binding?

Best,

Michal

I can see how this may be intentional, but body parser is included by default, as are the middlewares for it (Express.Middleware.json for example), but while Request.cookies etc are exposed, there is no Express.Middleware.cookie. It's a super simple binding, and including it would require adding cookieparser to the deps, so I can see where you may not want that, but it's a pretty small package and I think the odds of use are high enough to add the convenience.

Anytime I try to use "App.listen" passing more than two arguments I get this (super!) error.

12 │ App.listen app port::3001 onListen::(fun e => Js.log e) ();

  This function has type Express.App.t => port::int => unit
  It only accepts 2 arguments; here, it's called with more.
  Maybe you forgot a semicolon?

This is the only way I have been able to get it to work so far.

App.listen app port:3001;

If you wanted to use any middleware like https://helmetjs.github.io/ where in plain js you can do: app.use(helmet()).
Assuming that I have binded the helmet() part to ReasonML already would you do Routable.use(helmet()) to recreate the line app.use(helmet()) in plain js?

Hi, I didn't see anything in the example file or Express.re for getting headers out of a request. I can write my own code to do it but I wanted to check if I'm missing something.

express.Router is very similar to app but is designed to provide modularity. express.Router is especially useful when creating endpoints which are called from javascript. This is common use case for codebases which are gradually migrating to ReasonML.

I'm new to Reason/ReScript so appreciate any guidance.

Currently render() has the following type.

let render: (string, Js.Dict.t(string), 'a, t) => complete;

I.e. the data is a Js.Dict.t(string)

In the past when I've use Express I might have passed in data such as:

{
  user: { first: "Jo", last: "Smith" },
  products: [{ id: 1, name: "Product 1" }],
}

And the template would be crafted to handle those types, including looping over the products, for example.

What would be the correct strategy for handling types like those here?

Thanks

There are a handful of changes that haven't been published yet. It'd be great if you could bump the version!

Hello.
Im getting this error when trying to run the example.

example Error "Unbound value Request.asJsonObject"

i tried to run a stripped down version after like
index.re

open Express;

let app = express ();

App.listen app port::3000;

and its giving the error

Error: Unbound value express

i think im missing something obvious?

bsconfig.json

{
  "name": "hello",
  "version": "0.1.0",
  "sources": [
    "src"
  ],
  "reason" : { "react-jsx" : true},
  "bs-dependencies" : [
    "bs-express"
  ]
}

Hi, does bs-express support nested view rendering? For example, in Express it would be something like this:

res.render("user", { name: "Tobi" }, function (err, html) {
  res.render("default", { body: html });
});

Hi.

I can see that npmjs.org lists the latest version of this library as 1.0.2 and the code in master has "version": "1.0.2" since 4892d02:

Has the tag just been forgotten or is something more obscure happening?

Due to change in BS (https://bucklescript.github.io/blog/2019/09/23/release-5-20-0) local modules are no longer array but objects, which breaks compatibility when the bs-express library is used with npm i --production (for example under Google Cloud Functions)

PUT, PATCH and DELETE are endpoints commonly used when designing RESTful APIs.
Exposing App.put, App.putWithMany, etc. would make these bindings applicable to such a use case.

I read https://www.twilio.com/blog/lockfiles-nodejs a while ago.
And as I understand package-lock is useless when publishing a package to npm (as bs-express is). To not have conflicts between development and production versions maybe is better to ignore package-lock.json?!

Sorry for continuing to bother you about this! I see that the npm published 0.11.0 is missing lib/js. Did you publish manually or through CI? If it was published by CI I can play around with it to see if there's a problem with my previous PR.

bs-express is currently flagged as neglected on redex because the readme is severely inadequate. And the quality of the bindings is really not deserving of such a poor readme.

I suggest adding, at the very least, a description of what it is including a link to express for more information, an overview of the state it's in, what's implemented and what's not, and a usage example.

* nudge @ncthbrt *

Bindings for built in json, urlencoded middleware (available in Express v4.16.0 onwards)

currently the requirement of returning _done doens't work with async responses. Would be nice to be able to return e.g. Express.asyncMarker or something which is just let asyncMarker: done_ = [%bs.raw "null"] or something

Js.Json.reifyType is used twice in express.re. It was a deprecated function with bs-platform 1.9.0, but with the current 1.9.3 this function was removed.

The express Router function has several properties able to be set which modifies the routing behaviour. This should be exposed

Hi!

Many thanks for creating these bindings, I'm currently working on a project where I needed bindings for cookie-session.

Right now I've built and linked this package locally where I just naively added

let session: t => option(Js.Dict.t(Js.Json.t));

and

[@bs.get] [@bs.return null_undefined_to_opt]
external session : t => option(Js.Dict.t(Js.Json.t)) = "";

and put them below signedCookies in Express.re and Express.rei respectively which seems to work.

My bindings for cooke-session is currently very barebones, but could probably publish when I've added more as I go along.

Anyway, would you like me to create a PR to add this, or do you have other ideas?

Thanks,
Tom

@Et7f3 suggested rewriting https://github.com/reasonml-community/bs-express/blob/master/example/Index.re#L483 in Reasonml.

He pointed to https://github.com/TomLingham/bs-node-http/blob/master/src/Http.re#L102-L105.

I am not fully understanding how to solve this issue and write bindings but I would like to find out how!

@PeteProgrammer and @DrPandemic have proposed modifying this library to support piping. I think it's a great idea. It of course is a breaking change, but would likely improve the user experience, and AFAIK, there are no serious production users right now.

Example of how this would work (credit to @PeteProgrammer):

open Express;

module Response = {
  include Express.Response;
  [@bs.send.pipe : t] external status : int => t = "status";
  [@bs.send.pipe : t] external sendString : string => done_ = "send";
};

Usage:

Express.Middleware.from(
    (_, res: Express.Response.t, _) =>
        res |> Response.status(401) |> Response.sendString("Forbidden")
);

Hi there,
bs-express is looking for a new maintainer. I have been quite passively triaging issues and reviewing PRs, however I am not actively involved in the ReasonML community currently and so have not been keeping up with recent developments or changes to tooling and best practices.

I am happy to continue maintaining this repo in a passive manner, however I believe the project would be better served by someone who is willing to more actively take charge of the project and make more substantive improvements to the ergonomics or design of the library.

Natalie