|
1 | | -1. useStorage should be exported for plugin authors |
| 1 | +1. Plugins can't register their own UI |
2 | 2 |
|
3 | | -The recordings plugin has ~20 lines of boilerplate for storageKey(), loadStorage(), saveStorage() — reimplementing |
4 | | -the navigator*{namespace}*{instanceId} convention that useStorage already handles internally. If useStorage were |
5 | | -exported (like useUI and getMapInstance), the plugin shrinks significantly and the key convention stays |
6 | | -consistent. |
| 3 | +This is the biggest one. The Recordings feature requires three separate wiring points in the consumer's create() |
| 4 | +call: |
7 | 5 |
|
8 | | -2. Export a useMap() composable |
| 6 | +Navigator.create({ |
| 7 | +buttons: [{ id: 'record', component: RecordButton, panel: { component: RecordingsPanel } }], |
| 8 | +plugins: [RecordingsPlugin], |
| 9 | +}) |
9 | 10 |
|
10 | | -Components currently need two steps: inject('navigatorId') → getMapInstance(id). A useMap() composable that |
11 | | -resolves both internally would be more Vue-idiomatic and less error-prone. The internal composable already exists |
12 | | -— it just needs an external-facing export. |
| 11 | +The plugin manages all state, persistence, and map layers — but it can't declare its own button or panel. A |
| 12 | +self-contained plugin API would let you distribute a feature as a single import: |
13 | 13 |
|
14 | | -3. Document the @vitejs/plugin-vue prerequisite |
| 14 | +// Hypothetical: plugin registers everything itself |
| 15 | +plugins: [RecordingsPlugin] |
15 | 16 |
|
16 | | -The feature example shows .vue SFC files but doesn't mention that consumers need @vitejs/plugin-vue in their Vite |
17 | | -config. This was a build-breaking blocker with no clear error message. A note in the extending docs (or a Vite |
18 | | -plugin preset) would prevent this. |
| 17 | +The install() context could expose something like addButton() / addPanel() so a plugin can register its own UI |
| 18 | +components without the consumer manually wiring buttons config. |
19 | 19 |
|
20 | | -4. Document available SVG sprite icons |
| 20 | +--- |
21 | 21 |
|
22 | | -RecordButton.vue uses <use href="#record-btn-fill" /> — but there's no list of available icon names anywhere. |
23 | | -Plugin authors are guessing. A simple icon reference (even just a list) in the docs would help. |
| 22 | +2. useUI docs contradict the actual API |
24 | 23 |
|
25 | | -5. Add once to the plugin context |
| 24 | +The internal docs (4.ui.md) describe openPanel(id, component) and togglePanel(id, component) — taking an id and |
| 25 | +component as parameters. But the actual exported useUI has: |
26 | 26 |
|
27 | | -NavigatorInstance exposes once(), but the plugin install context only gets on/off/emit. Plugins that need one-time |
28 | | -setup (like map:ready) would benefit from once too. |
| 27 | +- openPanel() — no parameters, just opens the panel |
| 28 | +- setActivePanel(id) — separate function to set which tab is active |
29 | 29 |
|
30 | | -6. Add a plugin cleanup lifecycle |
| 30 | +The extending docs (8.extending.md) correctly show setActivePanel + openPanel as separate calls. The 4.ui.md docs |
| 31 | +should be updated to match. |
31 | 32 |
|
32 | | -There's no destroy/teardown hook for plugins. If the instance is unmounted, plugins can't clean up timers, |
33 | | -geolocation watches, or map layers. A on('destroy', fn) event or a return-value convention (install returns a |
34 | | -cleanup function) would prevent leaks. |
| 33 | +--- |
35 | 34 |
|
36 | | -7. Minimum version annotations in docs |
| 35 | +3. app.provide() boilerplate in plugins |
37 | 36 |
|
38 | | -useUI was added in 1.0.20, but the feature docs referenced it while the package was at 1.0.19. Adding @since |
39 | | -annotations to the API tables would save debugging time. |
| 37 | +Every plugin that shares state with Vue components must manually call app.provide('key', ...) and every component |
| 38 | +must inject('key'). The plugin context could offer a shorthand: |
| 39 | + |
| 40 | +// Current — manual provide |
| 41 | +install({ app }) { |
| 42 | +app.provide('recordings', { state, start, pause, ... }); |
| 43 | +} |
| 44 | + |
| 45 | +// Possible — context.provide() shorthand |
| 46 | +install({ provide }) { |
| 47 | +provide('recordings', { state, start, pause, ... }); |
| 48 | +} |
| 49 | + |
| 50 | +Minor, but it reinforces the pattern and makes plugins feel less like they're reaching into Vue internals. |
| 51 | + |
| 52 | +--- |
| 53 | + |
| 54 | +4. Per-instance plugins don't receive options |
| 55 | + |
| 56 | +Navigator.use(plugin, options) passes options as the second argument to install(), but per-instance plugins in the |
| 57 | +plugins array don't: |
| 58 | + |
| 59 | +// Global — options work |
| 60 | +Navigator.use(RecordingsPlugin, { maxDuration: 3600 }); |
| 61 | + |
| 62 | +// Per-instance — no options mechanism |
| 63 | +plugins: [RecordingsPlugin] // install() gets context only |
| 64 | + |
| 65 | +Supporting plugins: [{ plugin: RecordingsPlugin, options: { ... } }] (or a tuple syntax) would make per-instance |
| 66 | +plugins configurable without needing factory functions. |
| 67 | + |
| 68 | +--- |
| 69 | + |
| 70 | +5. @vitejs/plugin-vue is a hidden prerequisite |
| 71 | + |
| 72 | +The docs recommend Vue SFCs as the "preferred" approach for buttons and panels, but @vitejs/plugin-vue isn't |
| 73 | +mentioned until the very bottom of the extending docs. A consumer following the example will get an opaque Vite |
| 74 | +transform error. Options: surface this earlier in the docs, or detect the missing plugin at build time with a |
| 75 | +helpful error message. |
0 commit comments