Add a new public API exposed from the extension#5583
Conversation
This exposes a new API from the extension that provides access to the SDK paths/versions and DTD URI, including events that fire if/when they might change. Fixes #5008
DanTup
added
is enhancement
Well spotted, Copilot!
Good spot, fixed in ef85a18. |
|
/gemini review |
![gemini-code-assist[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new version of the public API to expose SDK paths and DTD URI, along with events for when they change. The implementation is clean and includes new tests. I've identified a potential unintended breaking change where a function from the previous API version was removed, and an outdated code comment. My feedback includes suggestions to address these points.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
This allows the exports to still work after an internal restart.
|
/gemini review |
There was a problem hiding this comment.
Pull Request Overview
Adds a new public API surface to expose SDK paths/versions and the DTD URI (with change‐notification events) from the Dart Code extension.
- Introduces
PublicDartExtensionApiinterface and implementation to emit SDK and DTD URI data/events - Updates
activateto set up and return the new public API object - Extends existing tests to import and verify the public API, and refines tooling‐daemon error messaging
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/helpers.ts | Import and export extApi for accessing the public API in tests |
| src/test/flutter_bazel/daemon.test.ts | Switch to named commands import and add reload command arg |
| src/test/flutter/extension.test.ts | Import extApi and assert public‐API SDK properties |
| src/test/dart/extension_api.test.ts | New tests for DTD URI and SDK‐change notifications |
| src/test/dart/extension.test.ts | Import extApi and assert public‐API SDK properties |
| src/shared/vscode/interfaces.ts | Allow toolingDaemon to be undefined in internal API |
| src/shared/services/tooling_daemon.ts | Improve error messages and refine send method behavior |
| src/extension/extension.ts | Initialize, update, and return the public API from activation |
| src/extension/api/interfaces.ts | Define the public API interfaces for SDKs and DTD URI |
| src/extension/api/extension_api.ts | Implement data‐backing and event emitters for the public API |
Comments suppressed due to low confidence (1)
src/test/dart/extension_api.test.ts:3
- [nitpick] Consider adding an assertion for
extApi.versionto ensure the public API version is covered by tests.
import { activateWithoutAnalysis, extApi, waitForResult } from "../helpers";
There was a problem hiding this comment.
Code Review
This pull request introduces a new public API for accessing SDK paths and the DTD URI, which is a great addition for other extensions to build upon. The implementation is well-structured, separating the internal data management from the public-facing API.
The changes to make DartToolingDaemon.send synchronous and throw on error is a nice simplification. The test coverage for the new API is also good.
I have one suggestion regarding a comment in extension_api.ts that seems to be out of sync with the implementation, which could cause confusion for future maintainers.
This exposes a new API from the extension that provides access to the SDK paths/versions and DTD URI, including events that fire if/when they might change.
Fixes #5008