Add Tab Functionality to the Plugins#25
Conversation
🦋 Changeset detectedLatest commit: 902f03b 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 |
There was a problem hiding this comment.
Code Review
This pull request introduces a new @sourceacademy/common-tabs package to define tab-related interfaces, refactors the web test plugin to register a test tab, and updates the build script to wrap external plugins in a module wrapper. The review feedback highlights three key areas for improvement: initializing module.exports as an empty object with an exports reference in the build wrapper to prevent runtime errors in standard CommonJS modules, explicitly importing React in the new tabs package to avoid TypeScript compilation issues for consumers, and prefixing the unused conduit parameter with an underscore in the runner test plugin to prevent lint warnings.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
8cce4e5 to
f4a1a7d
Compare
Akshay-2007-1
left a comment
There was a problem hiding this comment.
There are some minor changes in the infra.
One thing I wanna point out :
workspaceLocation prop is declared but never passed
src/web/test/src/index.tsx:428:
body: <Element tabService={tabService} id="test-tab" />,
workspaceLocation is an optional prop on Element but is never supplied here, so the rendered text "The location is {workspaceLocation}" will display as "The location is " (undefined).
This will not happen for actual tabs right? That also means that we have to pass the optional parameter workspaceLocation SURELY in the Element for it to be displayed correctly.
TestPlugin registers a tab but has no cleanup/destructor. If the plugin lifecycle includes teardown, this will leak the tab. If IPluginService has a destroy hook, unregisterTab should be called there.
When the tab is loaded, it's cloned and the When the evaluator changes, the tabs all get destroyed. I can make this clearer in the example web plugins interface if you wish |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The issue underpinning the migration of host-side plugins to external plugins is a requirement to display tabs on the frontend. In fact, most visualization plugins (the CSE machine, the Stepper, the Data Viz, etc.) require custom tabs. To retain the modularity gains provided by plugins, this PR aims to add a standardized interface to register, display, hide and unregister tabs.
This PR,
@sourceacademy/common-tabspackage, with theITabServiceandTabtypes.cjsand.jsexternal plugins to accept a custom require provider for thereact,react/jsx-runtime, etc. packages.Wiki Additions:
.jsformat