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.

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

v0.6.0 Beta version: New Interpreter and Compiler APIs #29

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open

Conversation

asg017
Copy link
Owner

@asg017 asg017 commented Apr 15, 2021

This is a WIP PR that changes the API based on #26. Wont merge until I add proper tests + documentation, but feel free to try it out and review! This is published as v0.6.0-alpha.0 to make it easier to test. Here are 2 notebooks where I'm using this new compiler:

  1. unofficial-observablehq-compiler v0.6.0 testing
  2. lets build an observable notebook editor

There are now separate Interpreter and Compiler classes. The interpreter can interpret cell and module source code, and returns variable definition to allow for easier deleting. I yeeted out the define/redefine separation of .cell, since it wasnt very helpful. I also prefer added a "config" parameter to pass in things like resolveFileAttachments, since I would forget the compiler's method signatures.

Let me know what you think!

@asg017
Copy link
Owner Author

asg017 commented Apr 15, 2021

Added tree shaking #28

@bryangingechen
Copy link
Contributor

Awesome, I'll try to take a look this weekend!

@gzuidhof
Copy link

gzuidhof commented Apr 16, 2021

This is fantastic, thank you.

In interpreter.js the following seem to be undefined:

AsyncFunction
GeneratorFunction
AsyncGeneratorFunction

In the old compiler.js you had the following

const AsyncFunction = Object.getPrototypeOf(async function() {}).constructor;
const GeneratorFunction = Object.getPrototypeOf(function*() {}).constructor;
const AsyncGeneratorFunction = Object.getPrototypeOf(async function*() {})
  .constructor;

Adding that back seems to solve the issue.

Somewhat related: have you considered maybe providing types, or using Typescript for the source code?

@asg017
Copy link
Owner Author

asg017 commented Apr 17, 2021

Thanks @gzuidhof , just put those functions back and republished as v0.6.0-alpha.2!

Re typescript, I tried that before, but since @observable/runtime and @observable/parser don't have any published types, it was a pretty big hassle (and Im not the best at TS). The codebase is fairly small, as well. I'd be happy to include any index.d.ts contributions!

@gzuidhof
Copy link

gzuidhof commented Apr 17, 2021

Awesome :) Here's an introductory notebook I'm (still) writing:
https://starboard.gg/gz/open-source-observablehq-nfwK2VA

I was wondering about the behavior of viewof x = Range(...), in Observable it does not show the value again underneath the slider. Is this something I'm doing wrong, or is this a difference in the interpreter? If it helps, here's the revelant bit in my integration.

As for types: you're right that Observable lacks types :(. I loosely typed some of the stuff I'm touching, perhaps we can spin that into a definition file at some point.

@bryangingechen
Copy link
Contributor

I was wondering about the behavior of viewof x = Range(...), in Observable it does not show the value again underneath the slider.

The editor on observablehq.com hides the value cell but it still exists as a separate "variable" under the hood. Hiding the values of viewof (and mutable) is something that needs to be done at the cell rendering level, not the compiler. It's been a while since I've worked with this stuff, but I think that in your code, before you call Inspector.into on line 93, you'll want to have logic to skip the rendering of the extra variables created by viewof and mutable.

@asg017
Copy link
Owner Author

asg017 commented Apr 17, 2021 via email

@bryangingechen
Copy link
Contributor

It's not documented yet, but I added a parameter to the compiler/interpreter to toggle whether or not it will "observe" the value cell for viewof (haven't done it for mutable yet, forgot about those). I think it needs to be done on the compiler level, since you don't know whether or not a cell is from a viewof cell (you only know the cell name and the value when it fulfills in the observer). https://observablehq.com/d/25c2ea3e6e6e81c7

Ah, I see. This approach does make rendering the whole notebook easier; I was expecting something closer to the Observable runtime API, where the inspector has to do all the work.

@asg017
Copy link
Owner Author

asg017 commented Apr 19, 2021

kk, so just rewrote and added a bunch of unit tests for the new Interpreter and Compiler classes. Also added a new observeMutableValues param to those classes (similar to observeViewofValues), re-wrote most of the documentation in the README, and took out a broken github action workflow. All has been published as v0.6.0-alpha.3.

There is still some documentation+tests that are needed for the .notebook methods, but this is starting to stabilize.

The sole unit test that I haven't converted yet is this one, which I hope to do before merging this in.

test("ES module: viewof + mutable", async t => {
  const compile = new compiler.Compiler();
  const src = compile.moduleToESModule(`viewof a = {
  const div = html\`\`;
  div.value = 3;
  return div;
}
mutable b = 3
{
  return b*b
}
d = {
  mutable b++;
  return a + b;
}
import {viewof v as w, mutable m} from "notebook"`);

  t.equal(src, `import define1 from "https://api.observablehq.com/notebook.js?v=3";

export default function define(runtime, observer) {
  const main = runtime.module();

  main.variable(observer("viewof a")).define("viewof a", ["html"], function(html)
{
  const div = html\`\`;
  div.value = 3;
  return div;
}
);
  main.variable(observer("a")).define("a", ["Generators", "viewof a"], (G, _) => G.input(_));
  main.define("initial b", function(){return(
3
)});
  main.variable(observer("mutable b")).define("mutable b", ["Mutable", "initial b"], (M, _) => new M(_));
  main.variable(observer("b")).define("b", ["mutable b"], _ => _.generator);
  main.variable(observer()).define(["b"], function(b)
{
  return b*b
}
);
  main.variable(observer("d")).define("d", ["mutable b","a","b"], function($0,a,b)
{
  $0.value++;
  return a + b;
}
);
  main.variable(observer()).define(
    null,
    ["md"],
    md => md\`~~~javascript
import {viewof v as viewof w, v as w, mutable m as mutable m, m as m} from "notebook"
~~~\`
  );
  const child1 = runtime.module(define1);
  main.import("viewof v", "viewof w", child1);
  main.import("v", "w", child1);
  main.import("mutable m", "mutable m", child1);
  main.import("m", "m", child1);
  return main;
}`);

  t.end();
});

@bryangingechen
Copy link
Contributor

Thanks for adding docs! I was getting a bit bogged down without them. I might not be able to come back to this until the coming weekend though, sorry!

@asg017 asg017 mentioned this pull request Apr 19, 2021
@asg017
Copy link
Owner Author

asg017 commented Apr 19, 2021

@bryangingechen not a problem, take your time, and thanks for all the help you've given! There are still some docs that I'll add before next weekend, and publishing as v0.6.0-alpha.x means we've got time. The one thing I'll need help with (if you have time!) is solving #27 for the Interpreter, since this can get pretty confusing...

@asg017 asg017 mentioned this pull request Apr 20, 2021
@Srabutdotcom
Copy link

Hi @asg017 ,

I tried to examine test("Interpreter: simple cells", ... by adding the following code , i mean i try to replace existing defined variable for example "b".

await interpret.cell("b = 4", main, observer);
t.equals(await main.value("b"), 4, "b are updated from 2 to 4");

but it is failed..??

@asg017
Copy link
Owner Author

asg017 commented Apr 21, 2021

@Srabutdotcom if your runtime module already has a cell named b, then running interpret.cell("b = 4") will fail with a b = RuntimeError: b is defined more than once error. You'll either have to redefine b on the runtime module specifically with module.redefine, or you can capture the returned variables from the previous interpret.cell("b = 1") call and run variable.delete on that.

See this notebook for details and code.

@Srabutdotcom
Copy link

@asg017

Here are my notebook using the help of your compiler,
take a look and give your feedback. aicone.id and klik on menu "notes"

Thanks

@davidbrochart
Copy link

Hi @asg017, I was wondering about the status of this PR, since it was never merged, but published as alpha versions.
I vendor it in ipyobservable, as I had to slightly change the code.
Also, I noticed that you pin @observablehq/parser to 4.2, I guess this means that the newer versions of the parser break your compiler?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants