observablehq / runtime

The reactive dataflow runtime that powers Observable Framework and Observable notebooks
https://observablehq.com/@observablehq/how-observable-runs
ISC License
1.02k stars 72 forks source link

Ability to observe imported cells without referencing? #245

Closed asg017 closed 4 years ago

asg017 commented 4 years ago

Basically a rehash of this forum post. It would be nice if the observer function was invoked for imported cells.

I tried to do this in a fork, by passing the observer parameter from module_import into the Variable constructor, but then this test fails:

# variable.import(name, module) does not compute the imported variable unless referenced
not ok 172 (unnamed assert)
  ---
operator: fail
    at: module.define (/home/runner/work/runtime/runtime/test/variable/import-test.js:29:51)
    stack: |-
      Error: (unnamed assert)
          at Test.assert [as _assert] (/home/runner/work/runtime/runtime/node_modules/tape/lib/test.js:225:54)
          at Test.bound [as _assert] (/home/runner/work/runtime/runtime/node_modules/tape/lib/test.js:77:32)
          at Test.fail (/home/runner/work/runtime/runtime/node_modules/tape/lib/test.js:318:10)
          at Test.bound [as fail] (/home/runner/work/runtime/runtime/node_modules/tape/lib/test.js:77:32)
          at module.define (/home/runner/work/runtime/runtime/test/variable/import-test.js:29:51)
          at /home/runner/work/runtime/runtime/src/runtime.js:231:33
  ...
not ok 173 should be equal
  ---
    operator: equal
    expected: false
    actual:   true
    at: _620‍.r.tape (/home/runner/work/runtime/runtime/test/variable/import-test.js:32:8)
    stack: |-
      Error: should be equal
          at Test.assert [as _assert] (/home/runner/work/runtime/runtime/node_modules/tape/lib/test.js:225:54)
          at Test.bound [as _assert] (/home/runner/work/runtime/runtime/node_modules/tape/lib/test.js:77:32)
          at Test.equal (/home/runner/work/runtime/runtime/node_modules/tape/lib/test.js:385:10)
          at Test.bound [as equal] (/home/runner/work/runtime/runtime/node_modules/tape/lib/test.js:77:32)
          at _620‍.r.tape (/home/runner/work/runtime/runtime/test/variable/import-test.js:32:8)
  ...
mbostock commented 4 years ago

This doesn’t require any runtime change—just a compiler change. For example, in @d3/bar-chart-remix, instead of

main.import("chart", child1);

the compiled code would need to say

main.variable(observer("chart")).import("chart", child1);
asg017 commented 4 years ago

Perfect, thank you so much! I was working on my own compiler, so I'll fix it there.

(closing since this has nothing to do with the runtime)