-
-
Notifications
You must be signed in to change notification settings - Fork 691
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
feat: Javascript Plugin API (Custom panels, column menu items with JS actions) #2052
feat: Javascript Plugin API (Custom panels, column menu items with JS actions) #2052
Conversation
…n names to clipboard
|
||
manager.registerPlugin("column-name-plugin", { | ||
version: 0.1, | ||
getColumnHeaderItems: (columnMeta) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd call this getColumnActions() to match with the names of other server-side action plugins: https://docs.datasette.io/en/stable/plugin_hooks.html#table-actions-datasette-actor-database-table-request
…for parity with Python API
Notes from 1:1 - it is possible to pass in URL params into a ObservableHQ notebook: https://observablehq.com/@bherbertlc/pass-values-as-url-parameters |
Javascript Plugin Docs (alpha)MotivationThe Datasette JS Plugin API allows developers to add interactive features to the UI, without having to modify the Python source code. SetupNo external/NPM dependencies are needed. Plugin behavior is coordinated by the Datasette There are 2 ways to add your plugin to the
const manager = window.__DATASETTE__;
document.addEventListener("datasette_init", function (evt) {
const { detail: manager } = evt;
// register plugin here
});
manager.registerPlugin("YOUR_PLUGIN_NAME", {
version: 0.1,
makeColumnActions: (columnMeta) => {
return [
{
label: "Copy name to clipboard",
// evt = native click event
onClick: (evt) => copyToClipboard(columnMeta.column),
}
];
},
}); There are 2 plugin hooks available to
While there are additional properties on the
I welcome ideas for more hooks, or feedback on the current design! ExamplesSee the example plugins file for additional examples. Hooks API Guide
|
Research notes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey! This is awesome: I really dig the pure vanilla JS plugin system (including the "InitDatasette"
custom event + window.__DATASETTE__
).
- Left some very nitty comments, not too important
- For the plugin methods
getColumnActions
andgetAboveTablePanelConfigs
: Maybe they should have more "action-y" names likeaddColumnActions
andaddAboveTablePanelConfigs
? It seems like theget*
prefix is there because they're lifecycle/hook names, but it seems odd to be to have a method calledget*()
that returns new items - I know TypeScript can be a little controversial, but for APIs like this (well defined callbacks and return objects) I find types to be extremely helpful. I think we can distribute an optional flat single-file
types.d.ts
file like so:
interface ColumnMeta {
column: string;
columnNotNull: boolean;
columnType: string;
isPk: boolean;
}
interface ColumnAction {
label: string;
onClick: (event: MouseEvent) => void;
}
interface AboveTablePanelConfig {
id: string;
label: string;
render: (node: HTMLElement) => void;
}
export interface Plugin {
version: number;
getColumnActions(columnMeta: ColumnMeta): ColumnAction[];
getAboveTablePanelConfigs(): AboveTablePanelConfig[];
}
export interface Manager {
plugins: Map<String, Plugin>;
registerPlugin(name: string, plugin: Plugin): void;
}
And then, one could use these types with JSDoc in plain JS files like so:
/**
* @typedef {import('./types').Manager} Manager
* @typedef {import('./types').Plugin} Plugin
*/
document.addEventListener("InitDatasette", function (evt) {
const { detail: manager } = evt;
addPlugins(manager);
});
/**
* @param {Manager} manager
*/
function addPlugins(manager) {
manager.registerPlugin("column-name-plugin", {
version: 0.1,
getColumnActions: (columnMeta) => { ... }
});
}
Then your editor will have type suggestions for manager
, columnMeta
, etc.
Probably not necessary for this PR, but happy to contribute a types.d.ts
when this is finalized!
- Maybe we can make it easier to do pure-js datasette plugins? For example the
example_js_manager_plugins.py
file is fairly small that just usesextra_js_urls
. Maybe there can be CLI options like:
datasette fixtures.db --plugin-js path/to/table-example-plugins.js`
And then do the PERMITTED_VIEWS
filtering in JS rather than Python. Or even autodetect JS files in --plugins-dir/
and server those automatically (though I doubt that's backwards compatible)
@@ -0,0 +1,202 @@ | |||
// Custom events for use with the native CustomEvent API | |||
const DATASETTE_EVENTS = { | |||
INIT: "InitDatasette", // returns datasette manager instance in evt.detail |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Maybe "DatasetteInit"
instead? Or "datasette_init"
? Not sure if there's a common convention for custom event names...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm flexible, I saw a bunch of opinions about this here:
https://stackoverflow.com/questions/19071579/javascript-dom-event-name-convention
It looks like it might be safe to err on the side of all-lowercase. datasette_init
or datasette.init
both have a reasonable look to them.
plugins: new Map(), | ||
|
||
registerPlugin: (name, pluginMetadata) => { | ||
if (datasetteManager.plugins.get(name)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: .has()
over .get()
* - column: string | ||
* - columnNotNull: 0 or 1 | ||
* - columnType: sqlite datatype enum (text, number, etc) | ||
* - isPk: 0 or 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could columnNotNull
and isPk
be booleans instead? Also, they appear to be strings right now ("0"
and "1"
rather than 0
and 1
, at least according to the "Log column metadata to console"
example
Also, maybe column
-> columnName
? or just name
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that makes sense, right now I'm passing through the data unparsed directly from the DOMStringMap
.
Probably more ergonomic to use these precise datatypes and more opinionated field names, great suggestions - I'll apply these soon.
*/ | ||
renderAboveTablePanel: () => { | ||
|
||
const aboveTablePanel = document.querySelector(DOM_SELECTORS.aboveTablePanel); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This returns null on tables that have no rows: ex in fixtures on http://localhost:8001/fixtures/123_starts_with_digits , I get:
caught TypeError: Cannot read properties of null (reading 'querySelector')
at Object.renderAboveTablePanel (datasette-manager.js:95:50)
at Object.registerPlugin (datasette-manager.js:51:24)
at addPlugins (table-example-plugins.js:34:11)
at HTMLDocument.<anonymous> (table-example-plugins.js:8:3)
at initializeDatasette (datasette-manager.js:193:12)
at HTMLDocument.<anonymous> (datasette-manager.js:201:3)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting- in those cases, do you think it still makes sense for the plugin to attempt to display (in which case there should be a fallback mount point, or the mount point should always show up regardless of whether there is data), or should a panel like this only show up for views that have some rows?
I think it's the first case, but wanted to check what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the thoughtful review and generous examples @asg017 ! I'll make the changes you suggested soon. Bonus thoughts inlined below.
These were very much appreciated, it's important to a plugin system that details like this feel right! I'll address them in batch later in the week.
FWIW I am in favor of doing Typescript - I just wanted to keep the initial set of files in this PR as simple as possible to review. Really appreciate you scaffolding this initial set of types + I think it would be a welcome addition to maintain a set of types.d.ts files. I'm entertaining the idea of writing the actual source code in Typescript as long as the compiled output is readable b/c it can be tricky to keep the types and plain JS files in sync. Curious if you have encountered projects that are good at preventing drift.
This is a great observation. I'm inclined towards something like
I really like this idea! It'll be easier to get contributors if they don't have to touch the python side at all.
One cost of doing this is that pages that won't use the JS would still have to load the unused code (given that I'm not sending up anything complex like lazy loading). But hopefully the manager core size is close to negligible, and it won't be a big deal. |
…fix to namespace instead of suffix
… action oriented verbs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm! Very eager to get this in, I want to add a hook for adding new features to the SQL editor with custom codemirror components
Thanks for putting this together! I've been slammed with work/personal stuff so haven't been able to actually prototype anything with this. :( tl;dr: I think this would be useful immediately as is. It might also be nice if the plugins could return The long version: I read the design notes and example plugin. I think I'd be able to use this in datasette-ui-extras for my lazy-facets feature. The lazy-facets feature tries to provide a snappier user experience. It does this by altering how suggested facets work. First, at page render time: Second, at page load time: there is some JavaScript that: With the currently proposed plugin scheme, I think (D) could be moved into the plugin. I'd do the ajax requests, then register the plugin. If the plugin scheme also supported promises, I think (B) and (C) could also be moved into the plugin. Does that make sense? Sorry for the wall of text! |
from datasette import hookimpl | ||
|
||
# Test command: | ||
# datasette fixtures.db --plugins-dir=demos/plugins/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be:
datasette fixtures.db \
--plugins-dir=demos/plugins/ \
--static static:demos/plugins/static
Then move datasette/demos/plugins/static/table-example-plugins.js
to demos/plugins/static/table-example-plugins.js
to get that to work.
// DATASETTE_EVENTS.INIT event to avoid the habit of reading from the window. | ||
|
||
window.__DATASETTE__ = datasetteManager; | ||
console.debug("Datasette Manager Created!"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was worried that there might be browsers in which this would cause an error (because console.debug
might not be defined), but as far as I can tell this has been supported in every modern browser for years at this point: https://console.spec.whatwg.org/ and https://developer.mozilla.org/en-US/docs/Web/API/console#browser_compatibility - so this is fine.
const datasetteManager = { | ||
VERSION: '1.0a2', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should make this available in the base.html
template, something like this:
<script>
datasetteVersion = '{{ datasette_version }}';
</script>
Then the datasette-manager.js
script, which is loaded later, can pick it up from there.
The {{ datasette_version }}
variable is already set and is used by the footer template here:
Powered by <a href="https://datasette.io/" title="Datasette v{{ datasette_version }}">Datasette</a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Brilliant! Confirmed that this works:
…ette version string
4b6fa80
to
eb1f408
Compare
Thanks for the review and the code pointers @simonw - I've made the suggested edits, fixed the renamed variable, and confirmed that the panels still render on the |
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2052 +/- ##
==========================================
+ Coverage 92.46% 92.69% +0.22%
==========================================
Files 38 40 +2
Lines 5750 6047 +297
==========================================
+ Hits 5317 5605 +288
- Misses 433 442 +9 ☔ View full report in Codecov by Sentry. |
This is such a well thought out contribution. I don't think I've seen such a thoroughly considered PR on any project in recent memory. |
Weird, the
|
Oh! I think I broke Cog on |
I'm landing this despite the cog failures. I'll fix them on main if I have to. |
Closes #2243 * Changelog for jinja2_environment_from_request and plugin_hook_slots * track_event() in changelog * Remove Using YAML for metadata section - no longer necessary now we show YAML and JSON examples everywhere. * Configuration via the command-line section - #2252 * JavaScript plugins in release notes, refs #2052 * /-/config in changelog, refs #2254 Refs #2052, #2156, #2243, #2247, #2249, #2252, #2254
Motivation
datasette-vega
,datasette-leaflet
, anddatasette-nteract-data-explorer
to co-exist safelyChanges
Summary: Provide 2 new surface areas for Datasette JS plugin developers. See alpha documentation
User Facing Changes
enter
to select.(plugin) Developer Facing Changes
datasette_init
CustomEvent when thedatasetteManager
is finished loading.manager.registerPlugin
API for adding new functionality that coordinates with Datasette lifecycle events.manager.selectors
map of DOM elements that users may want to hook into.table.js
to use refer to these to motivating keeping things in syncmakeColumnActions
: Add items to menu in table column headers. Users can provide alabel
, and eitherhref
oronClick
with full access to the metadata for the clicked column (name, type, misc. booleans)makeAboveTablePanelConfigs
: Add items to the panel. Each panel has a unique ID (namespaced within that plugin), a render function, and a string label.See this file for example plugin usage.
Core Developer Facing Changes
table.js
to make use of thedatasetteManager
API.demos/plugins
folder, and stored the test js in thestatics/
folderTesting
For Datasette plugin developers, please see the alpha-level documentation .
To run the examples:
Open local server:
http://127.0.0.1:8001/fixtures/facetable
Open to all feedback on this PR, from API design to variable naming, to what additional hooks might be useful for the future.
My focus was more on the general shape of the API for developers, rather than on the UX of the test plugins.
Design notes
makeColumnHeaderItems
benefits from hooking into the logic oftable.js
datasette-manager
would be more powerful if it were connected to lifecycle and JS events that are part of the existing table.js.Research Notes