Skip to content

Conversation

@texodus
Copy link
Member

@texodus texodus commented Oct 23, 2020

This PR removes @finos/perspective as a build-time dependency of @finos/perspective-viewer, which in practice means

  • perspective.wasm WebAssembly binary and WebWorker JS runtime are no longer packed with the @finos/perspective-viewer module, or any module that imports @finos/perspective-viewer. You must now also import @finos/perspective explicitly.
  • <perspective-viewer>'s load() method now only allows arguments of type Table(), which must be created explicitly from a WebWorker worker() or Websocket websocket().
  • Reciprocal Table() methods on <perspective-viewer> have been removed: replace(), clear(), update(). Use e.g. viewer.table.clear() instead.

@texodus texodus added this to the 0.6.0 milestone Oct 23, 2020
@texodus texodus marked this pull request as ready for review October 28, 2020 04:25
@texodus texodus requested a review from JHawk October 28, 2020 04:25
Copy link
Contributor

@sc1f sc1f left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PerspectiveWidget is currently broken on this branch - verified locally and through code review.

Intuitively, we should also make sure that PerspectiveWidget follows this API change - i.e. it should only __init__ or load with a table and not a dataset.

@texodus texodus force-pushed the simple-viewer branch 2 times, most recently from a61dfc3 to 6ab05fa Compare November 2, 2020 19:38
@texodus texodus requested review from sc1f and removed request for JHawk November 4, 2020 22:49
Copy link
Contributor

@sc1f sc1f left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! This should be merged before #1240 - there will be some conflicts on that PR (in the jupyterlab plugin and tests) that I will fix after this branch is merged.

*/
_update(data: TableData): void {
this.viewer.update(data);
this.viewer.table.update(data);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TypeError: this.viewer.table is undefined

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this introduces a bug as table isn't guaranteed to exist on the viewer. I don't think this was checked?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

4 participants