-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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(toolbar): Toolbar API improvements #10665
Conversation
🦋 Changeset detectedLatest commit: d719e40 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
id: string; | ||
name: string; | ||
icon?: Icon; | ||
}; | ||
|
||
export type DevToolbarApp = { |
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.
The types are quite messy around these parts right now because we support both patterns, once we remove the old pattern, there'll only be one type again
* feat(toolbar): Add helpers features * fix: consistent payloads and naming * chore: changeset * nit: rename eventTarget to app * feat: add server-side helpers * test: update test to use new APIs * fix: types * nit: erikaaaaa * feat: add new event * Update .changeset/khaki-pianos-burn.md * test: use data to create text * Apply suggestions from code review Co-authored-by: Florian Lefebvre <contact@florian-lefebvre.dev> * nit: use diff * nit: documentation effort * test: fix --------- Co-authored-by: Florian Lefebvre <contact@florian-lefebvre.dev>
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 PR is blocked because it contains a minor
changeset. A reviewer will merge this at the next release if approved.
!preview toolbar-improvements |
Snapshots have been released for the following packages:
Publish Log
Build Log
|
* feat: add a toolbar starter * test: skip examples that are not Astro projects * nit: small changes * feat: setup for build step * fix: add to devdep * docs: add commands to README
import type { DevToolbarApp } from '../@types/astro.js'; | ||
|
||
export function defineToolbarApp(app: DevToolbarApp) { | ||
return app; | ||
} |
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.
Since it's intended for external usage, and possibly build systems. I figured astro/toolbar
would be better, otherwise you can't build using TypeScript, you need to use external
etc. I'm expecting that we'll do the same with astro/integration
if we have exports there.
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! Just to double-check, is defineToolbarApp
exported from astro/toolbar
? The PR description shows from astro:toolbar
I knew I would forget to update it in one place somewhere 😔 Updated! |
!preview toolbar-improvements |
Snapshots have been released for the following packages:
Publish Log
Build Log
|
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.
A couple of thoughts on the changeset here for you, @Princesseuh ! 🙌
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
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.
Approving for docs!
Changes
This PR makes the surface smaller for defining a toolbar app and also introduce an helper function to define an app. These changes are intended to lower the barrier to create an app.
Before
After
Similar to
defineCollection
,defineConfig
etc,defineToolbarApp
takes care of the typing for you.The metadata is now part of the
addDevToolbarApp
call inside your integration, in addition of making the actual app code clearer since it only contains logic, this allows for better error handling in cases where your entrypoint is invalid, as we can now tell you exactly which app failed to load.This PR also includes new helpers and a new starter, see #10667 and #10793 for more details on that.
Testing
Modified the third-party plugin test to use those new APIs.
Docs
withastro/docs#7821