Skip to content
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.

Sign up for GitHub

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

Jump to bottom

BuckleScript JS bundle API rehaul 2020 #4450

Closed
6 tasks
ryyppy opened this issue Jun 8, 2020 · 1 comment
Closed
6 tasks

BuckleScript JS bundle API rehaul 2020 #4450

ryyppy opened this issue Jun 8, 2020 · 1 comment
Labels
playground

Comments

@ryyppy
Copy link
Member

ryyppy commented Jun 8, 2020 •
edited
Loading

This is a tracking issue for needed changes to build a good rescript-lang.org playground experience.
Since there have been substantial changes previously on the compiler infrastructure, this issue also replaces #3857.

Single window namespace

Right now, the bundle exposes two window objects: window.ocaml and window.reason, each of them implementing the same set of functions.

Our plan is to unify it to one single namespace instead, e.g. window.bs_platform (actual name needs to be discussed).

Reformatting Functionality

The BuckleScript bundle currently ships with Refmt_api which is capable of parsing / printing Reason code. We need access to these apis:

  • bs_platform.reason.parse(string): ast
  • bs_platform.reason.print(ast): string

Version Strings

  • bs_platform.reason.version: string (returns version from Refmt_Api)
  • bs_platform.ocaml.version: string (returns the OCaml version bs_platform is built on, e.g. 4.06.1)
  • bs_platform.version: string (returns the bucklescript version, such as v8.0.0)

Configuration

Right now there is no way to set any globally set configuration values within the JS BuckleScript compiler. Ideally we'd provide a function like bs_platform.setConfig(configObj) to set the configuration, optionally a function for setting separate values as well.

  • bs_platform.setConfig(ConfigObj)

whereas ConfigObj could look like:

type ConfigObj = {
  "module": "es6" | "commonjs" | "...",
  "bsc-flags": Array<string>,
  "use-super-errors": boolean
};

There's probably more, but for now having a way to configure the module output format would be a great boon for configuring different playground output formats.

With use-super-errors, we would be able to control the output format of the error messages as well.

Compilation

Right now, we offer different compilation functions with different sets of parameters, such as reason.compile(...) and reason.compile_super_errors(...).

With the new configuration functionality in place, we could configure the compiler depending on the user's preference (e.g if super-errors should be used) and expose a single reason.compile(codeStr) instead.
This was referenced Jun 8, 2020
Closed
Open
ryyppy added a commit to ryyppy/rescript-compiler that referenced this issue Jul 21, 2020
@ryyppy
Playground: First major refactor of the BS JS bundle API
0e309c6 
This commit introduces a breaking change to reorganize the API as
discussed in issue rescript-lang#4450.

The change will revolve around following features:
- Introduce a `bs_platform` namespace for all the functionality
- Reduce the global state of the compiler to allow different settings per instance
- Add configuration support to be able to manipulate the compiler setup
- Make super_errors reporting the default

**Introduce a bs_platform namespace**

Before we were independently adding language support via `window.reason`, `window.ocaml` and
`window.napkin`, etc. This behavior pollutes the global namespace, is harder to discover and
could potentially cause override conflicts.

With one unified `bs_platform` object, we can now group together all necessary functionality.

**Reduce global state / allow different settings**

We wanted to introduce different settings for different instances of the
bs_platform bundle API. The way the bundle was designed, the settings state
was fully dependend on the global compiler state, meaning that in case we
are using the bundle for things like multiple independent codesnippets within
a documentation page, everytime we are setting the compilation setting for one
snippet, it would automatically change it for all the snippets as well.

Therefore we needed to introduce a factory function that looks like this:

```
// load bs bundle globally via <script/> etc...
let bsc = bs_platform.make();

bsc.reason.setModuleType("es6");
bsc.reason.compile("let a = 1");
```

Internally, the API is still relying on one global compiler state, but we are now
able to locally differentiate between different setting profiles by injecting a
config record to each compile call.

**Make super_errors reporting the default**

Previously we offered functions like `ocaml.compile` and `ocaml.compile_super_errors` to
switch between different reporting engines. Problem was, that once you called `compile_super_errors`,
it would register the super_error reporters, and any further call with `use_super_errors=false` would
not take any effect, since the super_error reporters would not be removed (in fact, there are no apis to
clean up the changes of super_main.ml's setup function).

The goal is to make super_errors really solid, and it's probably the best default experience
for our users anyways. So to circumvent this global state issue by managing error reporters, we
just drop support for non super_errors behavior.
@ryyppy ryyppy mentioned this issue Jul 21, 2020
Closed
@ryyppy
Copy link
Member Author

ryyppy commented Jan 15, 2021

This issue is mostly obsolete in its design proposal. We refactored all the parts that were important for our current playground experience. The most up to date state of the API overhaul can be found in the the long running PR #4518.

@ryyppy ryyppy closed this as completed Jan 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
playground
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@ryyppy