Skip to content
This repository has been archived by the owner on Jun 15, 2023. It is now read-only.

Implement new syntax for importing values from JavaScript modules #217

Closed
wants to merge 7 commits into from

Conversation

IwanKaramazow
Copy link
Contributor

@IwanKaramazow IwanKaramazow commented Dec 30, 2020

New syntax for importing values from JavaScript

Motivation

Writing bindings to JavaScript has always been perceived as a rather daunting task. Most developers are not used to writing externals. If you look at TypeScript there's either something available on npm or you Any your way through the world. In ReScript it takes some practise to bring a library from npm at your fingertips. You have to learn to write an external and navigate through a sea of attributes. The team created the ReScript syntax with a very specific vision in mind. We want a familiar, yet expressive language. Struggling with JS bindings is not a part of this. Is there a way to make the life of our developers easier when it comes to importing values from JavaScript?

Basic example

// import values from a JavaScript module
import { delimiter: string } from "path"

Detailed design

Bindings to JavaScript values are currently done through a syntax based on the external keyword. Since it is a syntax for bringing in or importing JavaScript values, we move towards the import keyword. import is a known keyword in many languages: JavaScript, TypeScript, Java, Go, Swift, Haskell. The syntax for js imports will be available in both structures and signatures.

To increase familiarity for our developers, we can map the entire JavaScript import declaration syntax onto ReScript. This will greatly reduce friction for developers with a JS background. The one big difference is that ReScript import declarations require type annotations.

import { delimiter: string } from "path" // import named export from JS
import school: string from "school" // import default export JS
import  * as leftPad: (string, int) => string from "leftPad" // namespaced import, import entire module's content

Cases

Import a single export from a module

import { dirname: string => string } from "path"

// old external
@module("path")
external dirname: string => string = "dirname"

Import multiple exports from a module

import {
  delimiter: string,
  dirname: string => string,
  relative: (~from: string, ~to_: string) => string
} from "path"

// old external
@module("path")
external delimiter: string = "delimiter"
@module("path")
external dirname: (string, string) => string = "dirname"
@module("path")
external relative: (~from: string, ~to_: string) => string = "relative"

Import an export with a more convenient alias

import {renameSync as rename: (string, string) => string} from "fs"

// old external
@module("fs")
external rename: (string, string) => string = "renameSync"

import an export with an illegal ReScript identifier

import {"Fragment" as make: component<{"children": element}>} from "react"

// old external
@module("react")
external make: component<{"children": element}> = "Fragment"

import an export with additional attributes

import {
  @splice
  join: array<string> => string
} from "path"

// old external
@module("path") @splice
external join: array<string> => string = "join"

Rename multiple exports during import

import {
  renameSync as rename: (string, string) => string,
  unlinkSync as unlink: string => unit,
} from "fs" 

// old external
@bs.module("path")
external rename: (string, string) => string = "renameSync"
@bs.module("path")
external unlink: string => unit = "unlinkSync"

import an entire module's contents (aka NameSpacedImport)

import * as leftPad: (string, int) => string from "leftPad"

// old external
@module
external leftPad: (string, int) => string = "leftPad"

Importing default export

import schoolName: string from "/modules/school.js";
// syntactic sugar for
import {default as schoolName: string} from "/modules/school.js";

// old external
@module("/modules/school.js") external schoolName: string = "default"

Importing default export with illegal ReScript identifier

import "SchoolName": string from "/modules/school.js";
// syntactic sugar for
import {default as "SchoolName": string} from "/modules/school.js";

// old external
@module("/modules/school.js") external schoolName: string = "default"

importing default export with named imports

import schoolName: string, {getGrade: student => int} from "/modules/school.js";

// old external
@module("/modules/school.js") external schoolName: string = "default"
@module("/modules/school.js") external getGrade: student => int = "getGrade"

importing default export with multiple named imports

import
  schoolName: string,
  {
    getGrade: student => int,
    getTeachers: unit => array<teacher>
  }
from "/modules/school.js";

// old external
@module("/modules/school.js") external schoolName: string = "default"
@module("/modules/school.js") external getGrade: student => int = "getGrade"
@module("/modules/school.js") external getTeachers: unit => array<teacher> = "getTeachers"

importing default export with namespacedimport

import schoolName: string, * as school from "/modules/school";

// old external
@module("/modules/school.js") external schoolName: string = "default"
@module external school: student => int = "/modules/school.js"

import values from JavaScript modules with @as and polyvariants

import {
  openSync: (
    path,
    @bs.string [
      | @bs.as("r") #Read
      | @bs.as("r+") #Read_write
      | @bs.as("rs+") #Read_write_sync
      | @bs.as("w") #Write
      | @bs.as("wx") #Write_fail_if_exists
      | @bs.as("w+") #Write_read
      | @bs.as("wx+") #Write_read_fail_if_exists
      | @bs.as("a") #Append
      | @bs.as("ax") #Append_fail_if_exists
      | @bs.as("a+") #Append_read
      | @bs.as("ax+") #Append_read_fail_if_exists
    ],
  ) => unit
} from "fs"

// old external
@module("fs")
external openSync: (
  path,
  @bs.string
  [
    | @bs.as("r") #Read
    | @bs.as("r+") #Read_write
    | @bs.as("rs+") #Read_write_sync
    | @bs.as("w") #Write
    | @bs.as("wx") #Write_fail_if_exists
    | @bs.as("w+") #Write_read
    | @bs.as("wx+") #Write_read_fail_if_exists
    | @bs.as("a") #Append
    | @bs.as("ax") #Append_fail_if_exists
    | @bs.as("a+") #Append_read
    | @bs.as("ax+") #Append_read_fail_if_exists
  ],
) => unit = "openSync"

import a react component from a module

import {
  @react.component
  "Circle" as make: (
    ~center: Leaflet.point,
    ~radius: int,
    ~opacity: float,
    ~stroke: bool,
    ~color: string,
    ~children: React.element=?
  ) => React.element
} from "react-leaflet"

What does an interface file look like?

// Path.resi
import {delimiter: string} from "path"

Drawbacks

  • import will become a reserved keyword
  • Newcomers unfamiliar with ReScript's module system, might try to import generated JavaScript from the compiler. Behind the scenes you would be writing an external, pointing to the generated JavaScript.

Plan for adoption

The way we teach this is very simple:

  1. The printer will reformat code into the new syntax where possible.
  2. Ship a new version of the docs. Aligning the docs with a new release of the compiler containing this syntax will be crucial.

@yawaramin
Copy link

Will imported values be public members of the module, as they currently are with the external mechanism?

@illusionalsagacity
Copy link

This certainly does seem more ergonomic / familiar to me from the JS perspective. Perhaps this is just an oversight but I don't see an example of binding to variadic functions, common practice being binding to the same function with a different name for each kind of call. I assume this would be covered though like this?

type options

import {
  rmdirSync as rmdir: (string) => unit,
  rmdirSync as rmdirWith: (string, options) => string,
} from "fs"

Newcomers unfamiliar with ReScript's module system, might try to import generated JavaScript from the compiler. Behind the scenes you would be writing an external, pointing to the generated JavaScript.

Honestly this seems like a big deal -- I can easily see users new to ReScript not understanding why some things need to be imported but not others.

Also as noted making import a keyword would break (albeit trivially) anyone who bound to module.import for code-splitting? Are there concerns here for potential overlap with syntax-level future dynamic import support?

@TheSpyder
Copy link

Will imported values be public members of the module, as they currently are with the external mechanism?

Import is just syntax sugar for external, and one will format to the other, so the answer can only be yes 🤔

@gaku-sei
Copy link

gaku-sei commented Jan 1, 2021

I feel a bit mixed about that one to be honest 😕

While I do agree that the external system can be confusing for newcomers (and syntactically weird and verbose) from my experience it's not because of the wording but the binding system itself.

It seems the new import is basically sugar for external, but the way import works in JS and external works in ReScript are conceptually different. This will open the door to a bunch of new questions (additionally to the questions and concerns already mentionned), especially since when importing/binding a value ReScript doesn't always import it right away in the generated JS file, but on call site (for functions).

Plus, as of today, there is no import/export mechanics in ReScript at all making the new syntax potentially confusing for some people who already use the language.

Additionally to the above, what about the export keyword that's basically sugar for gentype? After that pr is merged import and export are not stricly symetric 😕

When speaking with people coming from JS/TS it seems the main issue is not the wording external/import or whatever, but to grasp the whole binding system, that doesn't exist in JS and works differently in TS (there is also a confusion ReScript module / Node module for some developers).

The binding logic exists in TS where you have a different extension (.d.ts), and the declare keyword for types for instance (Scala.js solves this with facade files as well).

@TheSpyder
Copy link

That's a much wider point than just commenting on the syntax change, and this is the wrong place for it. I suggest you post to the forum where it can be discussed properly 👍

@gaku-sei
Copy link

gaku-sei commented Jan 1, 2021

I don't think that's a "much wider point", this pr introduces a new syntax that i don't remember seing discussed much on the forum apart from a vague "we might introduce an import syntax" thrown in a blog post that didn't bring much details. And that syntax is meant to address the whole binding "issue".

Next step is a (very well explained) pr. There is no official annoucement or post yet on the forum, and my experience with ReasonMl/ReScript as an early adopter tells me that's better to be safe than sorry and raise concerns as soon as possible.

That being said, I'd be very happy to share my concerns on the forum if and when the core team is ready to open a post 😄

@TheSpyder
Copy link

From my reading of the blog post it's not intended to address the entire binding issue, only provide a more friendly syntax for certain scenarios (anything that doesn't involve @module will still use external syntax).

Regardless, this is definitely the wrong place to discuss general binding issues like the difference between JS import and ReScript export; that's why I suggested making a dedicated post about it.

@gaku-sei
Copy link

gaku-sei commented Jan 1, 2021

From my reading of the blog post it's not intended to address the entire binding issue

From my reading, it does. And that's exactly why i'm concerned: the situation, again, is not clear. What's the plan? Why a new syntax? What does it try to address if not the binding issue (and not only with Node modules/JS files)? Why an other "breaking change" ? What about the non module bindings ? If we keep external, why bother with import instead of improving the existing syntax for modules?

As for export there is already a post, and the feature seems to be confusing for some (which justifies my concerns).

Man, the way ReScript is managed really feels more and more like Elm to me....

@Kingdutch
Copy link

I would like to echo gaku-sei's sentiments.

I would also love to see an example of syntax for a .resi file. Some of my biggest hurdles have been trying to find out how to replicate the output of a PPX in a .resi file.

For example in the following example, what would an interface (.resi) file look like to only make renameSync visible outside of the module?

import {
  renameSync as rename: (string, string) => string,
  unlinkSync as unlink: string => unit,
} from "fs" 

Finally my initial impression of the new syntax was that it makes the message of explaining ReScripts (OCaml's) module system a lot more difficult. If this syntax existed when I started learning Reason/ReScript, my first attempt would be to try and import something from another .res file just as I would in JavaScript.

@illusionalsagacity
Copy link

illusionalsagacity commented Jan 1, 2021

Just spitballing here, what if import of a rescript/reasonml/ocaml file was just an alias for open / let assignments?

e.g.

import { bar } from "SomeReScriptModule" // --> let bar = SomeReScriptModule.bar

@IwanKaramazow
Copy link
Contributor Author

@yawaramin

Will imported values be public members of the module, as they currently are with the external mechanism?

Yes, public. It parses as external ….

@illusionalsagacity

I don't see an example of binding to variadic functions

You would import variadic functions with the @variadic attribute: https://rescript-lang.org/docs/manual/latest/bind-to-js-function#variadic-function-arguments. Added the example to the cases.

import {@variadic join: array<string> => string} from "path"

as noted making import a keyword would break anyone

It wouldn't break anyone. You can use keywords as regular identifiers: https://rescript-lang.org/docs/manual/latest/use-illegal-identifier-names

let \"import" = 3.14

Are there concerns here for potential overlap with syntax-level future dynamic import support?

This is an excellent question. There's still room in the syntax for import('/modules/my-module.js'). This can be made explicitly available when needed.

what if import of a rescript/reasonml/ocaml file was just an alias for open / let assignments?

The purpose of import is to import values from JavaScript. Unlike JavaScript, ReScript doesn't have or need import statements.

@Kingdutch

what would an interface (.resi) file look like

There would be no magic here, and you would do business as usual: (Added as example to the cases)

// Path.resi
import { delimiter: string } from "path"

@gaku-sei @Kingdutch, I hear you loud and clear. We want ReScript users to ship products, starting from a basic web app to a fully integrated SaaS solution built on JavaScript. Anything that we can do to help our developers ship, we will. Exploring new syntax to seamlessly interop between ReScript and JavaScript is one area where we can innovate and provide significant value for ReScript developers. This draft PR is a first step towards that vision. Regarding non module bindings, more research is needed and is high on my list. We definitely don't want to make things more difficult. One benefit of the new syntax parser is that it has been engineered with error messages in mind. So instead of <syntax error> we can actually link to good documentation and explain what went wrong. Importing ReScript from ReScript, is one example where we can show a nice error message and link to the docs. I am working closely with @ryyppy. We will document everything in detail.

Happy to answer more questions and best wishes for the new year!

Iwan added 4 commits January 3, 2021 15:41
@module("mat4") external create: unit => t = ""
1) attributes above import clauses, will be formatted above.
2) conversion: @bs.module now becomes automatically @module

All @module annotations also are parsed in first position. (like you would write them in front)
Updated the test data to have all @module annotations in first position.
Otherwise roundtrip tests wouldn't succeed.
@yawaramin
Copy link

@IwanKaramazow thank you. This is meaningfully different from the behaviour of ES's import statement, which brings the identifiers into scope only in the module and does not re-export them publicly. What do you think of instead using the export keyword? Both ES and TS use export to declare things that should be exported from the module. It would avoid the confusion of how it works and bypass any potential issues with people trying to import from ReScript sources.

@ryyppy
Copy link
Member

ryyppy commented Jan 4, 2021

@Kingdutch yes, there may be an initial learning curve for mistaking import for ReScript module imports... on the other hand, ppl will very quickly hit some walls (either don't know how what file to import, or they will realize that they need to add types to their imports, which will feel wrong). We'll have proper error messages, and hopefully, ppl will also read the import / export docs at some point? 😅

advantage of this particular syntax is that it is extremely easy to type and remember. Alternative would be to do @js import ...., which is gettin rather annoying after learning the differences?

@IwanKaramazow
Copy link
Contributor Author

@yawaramin if I'm reading this correct, you're proposing export {dirname: string => string, delimiter: string} from "path" to import values from JavaScript?

@Kingdutch
Copy link

Kingdutch commented Jan 4, 2021

Thanks for all the additional details. I think there may be a philosophical reason why it doesn’t quite sit right with me. I’ll try to explain.

I like ReScript (and formerly ReasonML/BuckleScript) because it doesn’t try to mix into JavaScript. TypeScript does blur the line with all sorts of annoyances as a result. Instead ReScript is a separate language. The boundaries are clearly defined and afterwards I can stop thinking about them.

This line between ReScript and JavaScript is clearly demarcated by the word “external”. I’m binding to something outside of ReScript. After that binding, I can stop thinking about it. From what I’ve seen in my own projects this encourages putting bindings in specific modules or packages and then not thinking about the JavaScript part.

What I am worried about with the new syntax is that this line is blurred and people are encouraged to start importing things all over the place. This works fine in the relatively simplistic examples given above but can cause major problems when multiple types of a module need to be passed around and start working together. Encouraging new people to use import instead of a module encapsulating external can cause duplicate incompatible type definitions throughout the codebase.

In summary I don’t feel like the syntactic sugar is enough of an improvement over the existing tools to warrant blurring the lines of separation of concern that are a great power in ReScript.

The printer will reformat code into the new syntax where possible.

Perhaps allowing people to disable this could alleviate the blurring of lines. Although I can see why you wouldn’t want to split codebases.

A complete alternative could be to make the import syntax a doc tool only. Write the code that transforms a valid JS import statement into an external declaration. Add that as a tool to the docs and people can copy and paste the output into their project.

———

There are two other considerations that I have: import overloading in JavaScript and FFI to non-js code.

To the first point: In JavaScript we see that modern platforms allow importing all sorts of things, like CSS, Images, etc. Will we support those things as well with the new syntax? How will the errors look for users when they try to import an image as a URL? Are we leaving ourselves room to add this in the future?

To the second point: one of the beautiful things for me, when I started out learning Reason, is that external just points to some external resource and the compiler figures out how to interop based on the context. In BuckleScript, annotations would make that an external to JS. In OCaml it could be to any other language.

This provides an interesting extension point because with the right PPX this could be extended to other external resources such as WASM or TypeScript.

@ryyppy
Copy link
Member

ryyppy commented Jan 4, 2021

@Kingdutch

I like ReScript (and formerly ReasonML/BuckleScript) because it doesn’t try to mix into JavaScript.

I'd state the exact opposite though... ReScript's success stems from the fact that we can seamlessly interact with JS without introducing a huge runtime / serialization layer. It would be really great to have the right language constructs to express zero-cost bindings and to allow quick JS interactions though.

What I am worried about with the new syntax is that this line is blurred and people are encouraged to start importing things all over the place.

It's more of a question why users import things all over the place (something that is already happening today btw, this is not related to simpler syntax).

I guess nobody imports stuff for fun, but more due to the fact that they cannot find any pure ReScript equivalents to library X, or they need to interact with their existing codebase (which will be very very likely). They probably also often need to interact with global apis such as the WebAPI, and currently it's terribly inefficient to write, read and teach those external bindings.

I think it's not less obvious that we are using unsafe code than with the external keyword, because we'll need to provide types for each import.. Some users might think we'll do some wrapping magic, but after they run into their first external related runtime error, or inspect the JS output (something we always adviced to this day), they will understand that we don't make it type-safe for them.

This line between ReScript and JavaScript is clearly demarcated by the word “external”.

Spotting the line shouldn't be harder than with externals. If I'd want to assess the amount of unsafe interop happening in my codebase, I'd regex search for the import keyword, just in the same way as with external. It's not like we are introducing an any type.. nothing has changed from a type safety perspective.

I'd be more worried about users advertising Obj.magic though 😄

This works fine in the relatively simplistic examples given above but can cause major problems when multiple types of a module need to be passed around and start working together. Encouraging new people to use import instead of a module encapsulating external can cause duplicate incompatible type definitions throughout the codebase.

Not sure how this is related to import in general? If I'd realize that one of my modules is reliant on type x that is introduced by via externals multiple times in different modules, I'd probably factor that out in a separate module, no? E.g. you could introduce a type Canvas.t and use it between different modules using import to interact with the Canvas api?

To the first point: In JavaScript we see that modern platforms allow importing all sorts of things, like CSS, Images, etc. Will we support those things as well with the new syntax?

Sure (as long as you configured your JS loader correctly):

import userImg: string from "/public/static/userImg"

// desugars to
@module("/public/static/userImg.jpg") external userImg: unit = "default"

How will the errors look for users when they try to import an image as a URL? Are we leaving ourselves room to add this in the future?

The syntax literally desugars to @module externals, so the import doesn't do any specific logic (therefore you can use urls or whatever you please, as long as you use the es6 output), and your JS loader is responsible for showing the right error if a user tries to import invalid resources.

@IwanKaramazow sticked with the ES6 import conventions, so I am pretty sure we are future proof.

To the second point: one of the beautiful things for me, when I started out learning Reason, is that external just points to some external resource and the compiler figures out how to interop based on the context. In BuckleScript, annotations would make that an external to JS. In OCaml it could be to any other language.

The external keyword will still exist, and the import syntax is just "an alias" to the right external decorations (@module). All the beauty will still persist, and there is no change in the compiler preventing that.

@TheSpyder
Copy link

@yawaramin if I'm reading this correct, you're proposing export {dirname: string => string, delimiter: string} from "path" to import values from JavaScript?

What if we come full circle and propose external {dirname: string => string, delimiter: string} from "path" 😂 or something like extimport 🤷‍♂️

In all seriousness, however this pans out it will improve life for me (I do a lot of module externals in my current project) and encourage us to switch to ReScript syntax.

I do agree with the sentiment here that matching the ES6 style exactly, while convenient for aesthetic reasons, has a lot of potential downsides. Some pros and cons off the top of my head:

  • 👎 High risk of confusion if it's explained as "write this like ES6 import syntax" when the semantics are very different
  • 👎 Having an import keyword that exports what it creates while open doesn't is confusing
  • 👍 Counter-point to the above: we already have include which does export what it adds
  • 👍 The syntax overlap reduces the amount of things to learn for a new user (i.e. they already know it, now it just works differently)

Overall I think sort of export/external alternate keyword would help, but import isn't a terrible choice.

@yawaramin
Copy link

@IwanKaramazow exactly, just swap import and replace with export.

@zth
Copy link
Contributor

zth commented Jan 4, 2021

@js import ... was a pretty nice suggestion in my opinion, although I'm unsure if @ryyppy meant it as a real suggestion 😄. Keeps the easy to remember syntax of import, and adds an extra tiny layer that clarifies what it actually is/does. I really like that personally.

@yawaramin
Copy link

@TheSpyder I know you were kidding, but I seriously thought about suggesting external {dirname: string => string, delimiter: string} from "path" :-) it has the advantage that it reuses an existing keyword.

@IwanKaramazow
Copy link
Contributor Author

@zth Yes, that was a real suggestion. The downside is that you now might occupy two lines:

@js
import {dirname: string => string} from "path"

The code looks nicer without the @js, but it might be clearer with? Maybe?

We can continue iterating on this later.
@zth
Copy link
Contributor

zth commented Jan 4, 2021

I think it's clearer with, even if it occupies two lines. That'd give a gentle nudge to the user that it's indeed an import from js, and I think that would reduce the potential confusion.

@TheSpyder
Copy link

@TheSpyder I know you were kidding, but I seriously thought about suggesting external {dirname: string => string, delimiter: string} from "path" :-) it has the advantage that it reuses an existing keyword.

I was only half kidding. It does neatly nod to what's really happening while still being almost entirely ES6 with one word swapped. But the moment we say "like ES6 import, but swap import to external" we start to run into the same documentation issues.

@zth Yes, that was a real suggestion. The downside is that you now might occupy two lines:

@js
import {dirname: string => string} from "path"

The code looks nicer without the @js, but it might be clearer with? Maybe?

Could you swap them?

import @js
{dirname: string => string} from "path"```

@bobzhang
Copy link
Member

bobzhang commented Jan 5, 2021

Let's not add import as a keyword here, import is used to import functions within the same language, also adding a new keyword adds lot of complexity, in general, we should avoid adding new keywords without introducing new semantics.

I suggest we do something more incremental as below:

external "fs" {
  rmdirSync as rmdir: (string) => unit,
  rmdirSync as rmdirWith: (string, options) => string,
} 

@gaku-sei
Copy link

gaku-sei commented Jan 5, 2021

I totally second @bobzhang 's idea here. Actually I was about to propose the same syntax with the external keyword 😄

To be honest I don't see any improvements using import instead of external as explained above 😕

On the other hand, I do like the new as syntax that makes bindings clearer and less fragile. Also, the new syntax should work for ambiant values.

To elaborate on @bobzhang 's example:

// Several bindings at once
external "fs" {
  rmdirSync as rmdir: (string) => unit,
  rmdirSync as rmdirWith: (string, options) => string,
} 

// One binding at a time
external "fs" rmdirSync as rmdir: (string) => unit

external "fs" rmdirSync as rmdirWith: (string, options) => string

// Global binding
external setInterval: (unit => unit, float) => unit

external setInterval as whateverCrazyName: (unit => unit, float) => unit

// Classes
type date

@new
external Date as date: unit => date

// Forbidden keywords would reuse the \"..." syntax
@new
external \"type" as type_: string

Or something along those lines.

It would:

  1. Makes bindings lighter and more robust (always felt a bit off to have = "importedValue" at the end of the external)
  2. Easy to learn/remember as it partly relies on the import syntax
  3. Easier to read
  4. Works well for all bindings (while some might need some @attributes)
  5. Nothing new to learn

@ryyppy
Copy link
Member

ryyppy commented Jan 5, 2021

hmm, before we get this kind of syntax, I'd rather just have no changes. Less work on the syntax and a lot less to do educational / docs wise.

What does external "abc" even mean? that's complete off from a left to right reading perspective and looks like you are defining an external called abc.

At least keep the "from" keyword to make it somewhat understandable.

Messing around with two completely twisted kinds of "import syntaxes" when dealing with JS / ReScript codebases will just continue to be a huge pain.

Also, probably worth mentioning: import is already a reserved keyword today... you need to escape it, even if the keyword is not used.

@gaku-sei
Copy link

gaku-sei commented Jan 5, 2021

before we get this kind of syntax, I'd rather just have no changes

That sounds like the most reasonable thing to do to be honest 😕
Introducing the new import syntax is just more confusing than anything, and doesn't solve any pain points.

From the look of it, there are 2 pain points:

  • The newcomers who have a hard time to understand the binding syntax (later on that one). But it doesn't seem that adding yet an other construct with import will make the complexity vanish, on the contrary it'll add complexity on top of complexity. Especially as it seems from a previous post that import would be used additionally to external. Again, not only is it a bit weird to use import to not import anything, but also it's not symetric with the way export currently works 😕 (the reason why export is sugar for @genType is beyond me, but that's another topic).
  • The people who have to write a lot of externals and would like a more lightweight/clearer syntax

Also, and while I understand ReScript is not the same as TS/Scala/C#/etc... it's always worth taking a look at how things are achieved in other languages. In TS the module binding system exists, and it's a .d.ts declaration file, Kotlin uses external, Scala.js uses something very close (with some annotations), so the concept is not that foreign, and shoudn't be a big hurdle for newcomers (and from my experience working with newcomers no one had a hard time with the external syntax, but rather the "proper" way to bind some kind of values, and to remember all the @ keywords, but that's an other issue, and import doesn't solve it either).

Taking the TS way, we could even consider using the resi files to bind js files when there is no equivalent res file, but that could be even more confusing. Adding yet an other extension like resd, using the same syntax as resi but binding external values is also an idea, and it would feel very familiar to TS developers.

that's complete off from a left to right reading perspective

Hey!! That's how it works on Python and it seems ok for many developers 😆

@jchavarri
Copy link
Contributor

Introducing the new import syntax is just more confusing than anything, and doesn't solve any pain points.

I strongly disagree with this. Writing bindings has historically been one of the most mentioned frictions when onboarding new users, mostly because current approach has nothing to do with JS way to import code, so one has to learn some random attributes like bs.new or whatever. In that sense, I very much appreciate this proposal, and I believe it is valuable because it brings bindings syntax closer to what JS users are used to already.

I also agree that (ab)using import for this case is confusing, but I also think (ab)using external to then reinvent totally different semantics (that have nothing to do with OCaml external) is equally confusing.

What if there was a totally different word used for this? e.g. use:

use { delimiter: string } from "path"
use school: string from "school"
use * as leftPad: (string, int) => string from "leftPad"

That way there is no room from confusion, but users still can leverage syntactic knowledge about how values are imported in JS thanks to the proposal.

@pckilgore
Copy link

pckilgore commented Jan 5, 2021

I strongly disagree with this. Writing bindings has historically been one of the most mentioned frictions when onboarding new users, mostly because current approach has nothing to do with JS way to import code, so one has to learn some random attributes like bs.new or whatever. In that sense, I very much appreciate this proposal, and I believe it is valuable because it brings bindings syntax closer to what JS users are used to already.

I'd like to second this almost in its entirety. This is a good thing to talk about. I want to thank the original poster and those they consulted for their efforts tacking "the bindings problem". I would also like to observe that in the future there are fewer sunk costs and less ego involved with feedback on a proposal versus an implementation. Perhaps I missed the original proposal? Perhaps even a simple established RFP procedure would alleviate this in the future?

When I hit my personal wall with bindings, it was not the syntax that confused me (other than = "someString" being a small speed bump). It was typing, and understanding what Rescript/Reason wanted me to do. The problem was literally what to type as the type, particularly for objects, polymorphism, when this abstract type thing was necessary, etc. I believe this is what @jchavarri notes when he points out the pain involved with bs.new bs.variadic and friends.

It seems to me like a new syntax does nothing to fix that actual pain, yet incurs all the typical costs of a more complicated (larger) language syntax (with the added pain of having two language keywords to express the same concept). In addition, it incurs the overhead of asking new reason users to unlearn a concept from javascript. For example, as a new user to reason, one of the very first things that confused me was "how do I reference something in this other file". Should I have been presented with an import syntax, you bet your butt I would have tried to use it, because that's what I knew to use for javascript. And while we could easily throw a yellow box up on a docs page explaining this difference at the top, I would gently suggest adding new syntax that immediately wants for a warning that it does not do what you think it does, is not ideal.

In sum, I would question the underlying assumption that syntax is the problem (but am open to being shown otherwise). That being the case, I think the problems with any new keyword might outweigh the benefits, and would prefer it not be added until we can establish that people are really being tripped up on the syntax of externals versus the deeper issues around type modeling a dynamic language with ml.

FWIW, I found Typescript declaration files extremely esoteric when I learned Typescript (and still think so in some respects, lol). But it is a non-issue since coverage is good with with definitely typed. Are there any immediate problems with going that route instead: having a centralized, easy to search, easy to contribute repository of bindings to which we can refer new community members to for examples of bindings and pre-written bindings? Not every solution to every problem is an engineering one, here documentation, advocacy, or a product like a definitely typed for rescript might help address the same issue as this PR

@cometkim
Copy link

cometkim commented Jan 5, 2021

As a ReasonML/ReScript beginner, what I felt while writing some bindings, syntaxes/keywords weren't really obstacles, but It made it easy to infer that the different behavior.

The reason why binding became difficult for me was to write bindings for the point-free style abstraction, and for this, I had to abandon the zero-cost binding many times.

In fact, these courses have helped me a lot in learning the reason-ish way. So I think it's a much more helpful approach that to provide a binding template like flow-typed does or a converter like ReasonablyTyped in case users are sometimes confused about how to achieve binding properly.

@wokalski
Copy link

wokalski commented Jan 5, 2021

If I understand correctly the problems the people pointed out with the proposed syntax are:

  • different semantics than JS (it imports and exports at the same time by default)
  • Polluting keyword space

as previously mentioned the import here closely follows the semantics of include. Therefore I think we can have a self explanatory import-like mechanism (hopefully not too difficult to parse) with:

include { delimiter: string } from “path”

@gaku-sei
Copy link

gaku-sei commented Jan 6, 2021

@jchavarri

Introducing the new import syntax is just more confusing than anything, and doesn't solve any pain points.

I strongly disagree with this.

I may have poorly explained my point of view here (especially because as it appears in your comment, we agree on many points 😄 ).

I do think an update of the syntax is desired, and could solve some of the issues listed here, here, and there. But import sounds really confusing as it basically doesn't import anything, that's it. Words have meaning, and using import just for the sake of familiarity is probably not a great choice, but again, that's just my point of view after trying to understand the binding system myself, and working regularly with new comers to the language.

so one has to learn some random attributes like bs.new or whatever

Agree, that's, to me, the main pain point when learning the external (with the expected type), but import doesn't solve it. It only drops the @module annotation, that's it (and that's probably the less confusing part of the external system 😄). It seems external and the other @something will still be around for a while according to @ryyppy here (last paragraph).

But again, and even though I totally agree a small syntax update would be very welcome (less annotations, less confusion regarding the = "???" syntax, etc...), let's keep in mind the syntax is clearly not the biggest obstacle in the learning curve of externals.


Slightly unrelated, and while @ryyppy and other team members are currently doing a great job with the documentation, one of the biggest issue when binding a value used to be the lack of clear documentation, with case by case examples (there was no official and curated cheatsheet for annotations for a looooong time for instance).

@jfrolich
Copy link

jfrolich commented Jan 6, 2021

Introducing the new import syntax is just more confusing than anything, and doesn't solve any pain points.

I strongly disagree with this. Writing bindings has historically been one of the most mentioned frictions when onboarding new users, mostly because current approach has nothing to do with JS way to import code, so one has to learn some random attributes like bs.new or whatever. In that sense, I very much appreciate this proposal, and I believe it is valuable because it brings bindings syntax closer to what JS users are used to already.

I also agree that (ab)using import for this case is confusing, but I also think (ab)using external to then reinvent totally different semantics (that have nothing to do with OCaml external) is equally confusing.

What if there was a totally different word used for this? e.g. use:

use { delimiter: string } from "path"
use school: string from "school"
use * as leftPad: (string, int) => string from "leftPad"

That way there is no room from confusion, but users still can leverage syntactic knowledge about how values are imported in JS thanks to the proposal.

I strongly agree with this. But please don't make use a keyword, because in our codebase we have a pattern with a module and a use function as a React hook (for instance let dimensions = WindowDimensions.use().

@jfrolich
Copy link

jfrolich commented Jan 6, 2021

If I understand correctly the problems the people pointed out with the proposed syntax are:

  • different semantics than JS (it imports and exports at the same time by default)
  • Polluting keyword space

as previously mentioned the import here closely follows the semantics of include. Therefore I think we can have a self explanatory import-like mechanism (hopefully not too difficult to parse) with:

include { delimiter: string } from “path”

Love this! 👆 We don't lose a keyword (or use a confusing one like import) and it is pretty self explanatory (the other include also exposes the contents outside the module.

@Kingdutch
Copy link

Great to see all the discussion that's developed over the past days. Most capture what I've been mulling over away from this thread quite well. I think I'm left with two main thoughts.

First, moving away from import to something else solves most of my worries in that regards, since you don't bring the baggage that people associate with that keyword from JavaScript and break that behaviour, but make it clear that you're introducing something new.

Since we're constantly talking about making 'bindings' is there a reason we haven't considered the bind keyword yet? i.e bind { delimiter: type } from "path"? The advantage is that if you type in rescript bind you'll probably already get existing documentation and articles for bindings? The only downside I can come up with is that it's an existing JavaScript function name.

Secondly, so far I feel like we're introducing a syntax that only covers parts of the issues. This has been mentioned before. I share the feeling that the syntax actually only improves the easiest parts of the syntax. Removing the annotations that I struggled with the least when I was a newcomer to Reason. Some of the more difficult parts are left intact.

The worry here is that the documentation moves to the new syntax and leaves users stranded if that syntax isn't adequate forcing them to use the external syntax, which is then less likely to be documented well. The most notable example is probably "import values from JavaScript modules with @as and polyvariants". That actually taught me some things I haven't been able to figure out before.

(I have to note that before writing this I did a cursory search of the ReScript docs and am now able to find examples of most of the decorators that I wasn't able to find in previous searches. I'll see if I can contribute by linking the list in the cheatsheet to the documentation for each decorator.)

@bobzhang
Copy link
Member

bobzhang commented Jan 7, 2021

The worry here is that the documentation moves to the new syntax and leaves users stranded if that syntax isn't adequate forcing them to use the external syntax, which is then less likely to be documented well.

That's why I suggest incremental improvement for most common use cases

@jchavarri
Copy link
Contributor

jchavarri commented Jan 8, 2021

@bobzhang @IwanKaramazow @ryyppy Maybe this have been discussed somewhere, but how does the proposal in this PR fit with the other (and I believe more ambitious) proposal of having a JavaScript-like DSL for FFI that was discussed in rescript-lang/rescript-compiler#3618?

That proposal (in particular, the modification added by @glennsl) was really compelling from a teaching / onboarding newcomers perspective, as it removes the need to learn a list of new custom attributes and leverages the knowledge of JS syntax new users will most probably have already.

If it is still planned, an "external syntax roadmap" would be valuable in order for everyone to visualize and understand the path to progressively move towards the DSL for FFI end goal. Also, if that DSL for FFI would eventually happen, then I'd tend to agree with the comments above that adopting this proposal would have to be better justified, to not pile on additional syntax changes, which I believe the community is still recovering from.

@pckilgore
Copy link

FWIW going back over this I really like @bobzhang 's incremental proposal here: #217 (comment)

It will keep any current docs and blogs about externals relevant to the problem, remove the ugliest parts of the current syntax, and given time we might just find there is no real need for a new keyword.

@jacobp100
Copy link

If you wanted to do the incremental approach in #217 - what if we tried to match the typescript semantics?

declare module "fs" {
  export let rmdirSync: (string) => unit;
}

For exporting with a different type signature and name - I'm not 100% sure what we do, but this is valid TS -

declare module "fs" {
    let rmdirSync: (string) => unit;
    export {rmdirSync as rmdirWith};
}

I don't fully know how that would work to do what we want to do though

@bobzhang
Copy link
Member

bobzhang commented Feb 1, 2021

for the reference, we had similar discussions before: rescript-lang/rescript-compiler#2067 (comment)

@stale
Copy link

stale bot commented May 28, 2023

The rescript-lang/syntax repo is obsolete and will be archived soon. If this issue is still relevant, please reopen in the compiler repo (https://github.com/rescript-lang/rescript-compiler) or comment here to ask for it to be moved. Thank you for your contributions.

@stale stale bot added the stale label May 28, 2023
@stale stale bot closed this Jun 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet