-
-
Notifications
You must be signed in to change notification settings - Fork 196
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
Browser plugin preview #2748
Browser plugin preview #2748
Conversation
|
…onnected browser dev preview)
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.
nice overall! let's hold off merging this until 2.0 is out, too much risk (and its the basis for devex anyway)
@@ -26,6 +26,50 @@ export type AsyncMessageChannelHandlers = { | |||
> | |||
}; | |||
|
|||
const WEBSOCKET_SERVER_URL = 'ws://localhost:9001/ws'; |
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 we move this to a dedicated file? as in just import those functions here - to keep things cleaner?
packages/tokens-studio-for-figma/src/app/asyncMessageHandlers/preview.tsx
Outdated
Show resolved
Hide resolved
packages/tokens-studio-for-figma/src/app/asyncMessageHandlers/preview.tsx
Outdated
Show resolved
Hide resolved
packages/tokens-studio-for-figma/src/app/asyncMessageHandlers/preview.tsx
Outdated
Show resolved
Hide resolved
packages/tokens-studio-for-figma/src/app/asyncMessageHandlers/preview.tsx
Outdated
Show resolved
Hide resolved
packages/tokens-studio-for-figma/src/app/asyncMessageHandlers/startup.tsx
Outdated
Show resolved
Hide resolved
const possiblePromise = callback(event.data.pluginMessage); | ||
if (possiblePromise === false || (possiblePromise && await possiblePromise === false)) { | ||
window.removeEventListener('message', listener); | ||
private startWebSocketConnection() { |
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.
lots of these functions feel like we should move them to either a shared websocket file or their own files alltogether in order to improve maintainabiltiy
const possiblePromise = callback(msg); | ||
if (possiblePromise === false || (possiblePromise && await possiblePromise === false)) { | ||
figma.ui.off('message', listener); | ||
switch (this.environment) { |
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.
any way where we can make this function overall more readable? lots of nested ifs and switch statements 🤔
Co-authored-by: Jan Six <[email protected]>
…t now) Co-authored-by: Jan Six <[email protected]>
Co-authored-by: Jan Six <[email protected]>
Co-authored-by: Jan Six <[email protected]>
packages/tokens-studio-for-figma/src/AsyncMessageChannelPreview.ts
Dismissed
Show dismissed
Hide dismissed
Commit SHA:409d986497c81348ca28f85565525d40d5506dc4 Test coverage results 🧪
|
Commit SHA:409d986497c81348ca28f85565525d40d5506dc4 |
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.
NIIICE!
General questions for my understanding, and a couple of code simplification suggestions. Let me know what you think!
I haven't played around much with it in the browser, will add any comments if there's breakages.
@@ -11,6 +11,8 @@ | |||
"build:dev": "cross-env NODE_ENV=development webpack --mode=development", | |||
"build:cy": "cross-env LAUNCHDARKLY_FLAGS=tokenThemes,gitBranchSelector,multiFileSync,tokenFlowButton yarn build", | |||
"start": "cross-env webpack --mode=development --watch", | |||
"preview:plugin": "cross-env webpack --mode=development --PREVIEW_ENV=figma --watch", | |||
"preview:browser": "cross-env WEBSOCKETS_PORT=9001 node preview-server.js & webpack-dev-server --mode=development --PREVIEW_ENV=browser", |
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.
Can we add descriptions on the README
(or sibling doc in /developer-knowledgebase
) on how to run the plugin in the browser?
Similar to the PR description, stating the order of commands and possible hiccups like I had 🙈
app.use(express.static(__dirname + '/dist')) | ||
|
||
app.get("/", (req, res) => { | ||
// res.sendFile(path.join(__dirname, 'dist', 'index.html')); |
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.
Accidentally commented out?
}); | ||
|
||
// send immediatly a feedback to the incoming connection | ||
// ws.send('Hi there, I am a WebSocket server'); |
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.
Can this be updated or removed?
@@ -142,3 +144,7 @@ export class AsyncMessageChannel { | |||
return promise; | |||
} | |||
} | |||
|
|||
const ExportedAsyncMessageChannel = !process.env.PREVIEW_ENV ? AsyncMessageChannel : AsyncMessageChannelPreview; |
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.
const ExportedAsyncMessageChannel = !process.env.PREVIEW_ENV ? AsyncMessageChannel : AsyncMessageChannelPreview; | |
const ExportedAsyncMessageChannel = process.env.PREVIEW_ENV ? AsyncMessageChannelPreview : AsyncMessageChannel; |
Could we turn the ternary around? Feels weird having a negation to start with.
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.
Out of curiosity: what are the possible values of process.env.PREVIEW_ENV
? browser
| figma
...?
// credits goes to https://github.com/microsoft/TypeScript/issues/23182#issuecomment-379091887 | ||
type IsTypeOnlyObject<Obj extends Record<PropertyKey, unknown>> = [keyof Obj] extends ['type'] ? true : false; | ||
|
||
type IncomingMessageEvent<Message = unknown> = { | ||
data: { | ||
pluginMessage: { | ||
id: string | ||
message: Message | ||
} | { | ||
id: string | ||
error: unknown | ||
} | ||
} | ||
}; | ||
|
||
export type AsyncMessageChannelHandlers = { | ||
[K in AsyncMessageTypes]: (incoming: AsyncMessagesMap[K]) => Promise< | ||
IsTypeOnlyObject<AsyncMessageResultsMap[K]> extends true | ||
? void | ||
: Omit<AsyncMessageResultsMap[K], 'type'> | ||
> | ||
}; | ||
|
||
const WEBSOCKET_SERVER_URL = 'ws://localhost:9001/ws'; |
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.
These are the same types as used AsyncMessageChannel
. Why not create a sub-folder containing this file + its counterpart, and perhaps a shared.ts
file with the shared types / constants?
Saves a bit of duplication.
import { INTERNAL_THEMES_NO_GROUP } from './constants/InternalTokenGroup'; | ||
import { | ||
AsyncMessageTypes, GetThemeInfoMessageResult, | ||
} from './types/AsyncMessages'; | ||
|
||
describe('Testing the mock functionality of the AsyncMessageChannel', () => { | ||
it('should be able to communicate between UI and plugin', async () => { | ||
const runAfter: (() => void)[] = []; | ||
const runAfter: ((() => void) | null)[] = []; |
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 save () => void) | null
as a type and avoid having to update it everywhere if it changes!
module.exports = (env, argv) => { | ||
const wrapper = (callback) => { | ||
const measureSpeed = false; | ||
if (!measureSpeed) { // Set to true to enable SpeedMeasurePlugin (breaks Figma UI build) |
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 this comment be more thorough?
If not here, perhaps in a doc section to go through the why's and how's 😄
|
||
``` | ||
|
||
- |
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.
Accidental bullet point
E -->|"ReactInstance.handle(...)"| D | ||
E -->|"sendMessageToBrowser\n(ws.send(...))"| F | ||
F -->|"sendMessageFromBrowser\n(ws.send(...))"| E | ||
``` |
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.
Great diagram!!! 🎉
Static instances of `AsyncMessageChannel` are initialised when the class is loaded: | ||
|
||
- `PluginInstance` - used from inside of `controller` entrypoint | ||
- `ReactInstance` - used from inside of `ui` entrypoint |
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.
Worth mentioning what inFigmaSandbox
means?
|
* fix AsyncMessageChannelPreview undefined error + export WS URI * browser preview CSS file for UI fixes * create previewUtils for browser color scheme + setFigmaBrowserTheme * browser preview URL params + fullscreen/theme/action modes * two bug fixes for browser/plugin websocket preview bridge * add preview dist folder for web preview builds * [WIP] browser preview dev knowledge docs * feat(dev): request startup on browser preview page open * refactor(dev): use env vars for browser preview ws src * fix(debug): remove console.log from asyncmessagechannelpreview * fix(css): figmaloading full height css for browser preview * refactor(dev): use enums for websockets src in browser preview tsx * fix(dev): remove comments * refactor: reuse htmlClassList variable * remove unused package --------- Co-authored-by: macintoshhelper <[email protected]>
* [WIP] webpack config setup for fast refresh + websocket server * add prod/dev hmr webpack config option * render immediately and handle AsyncMessage in startup app hook * forward ui AsyncMessages to browser via WebSockets * null check sentry transaction to fix browser error * refactor AsyncMessageChannel code for browser implementation * webpack use swc-loader for browser version + speed/bundle size plugin options * prettify browser dev preview UI * enable loading screen if startup params missing (for web serve + disconnected browser dev preview) * attempt to fix webpack build for tests * add SpeedMeasurePlugin package * create AsyncMessageChannel dev docs * replace web-preview.md ASCII data flow diagram with mermaid * use radii/spacing tokens instead of px for web preview.tsx styles Co-authored-by: Jan Six <[email protected]> * remove commented out startup handler (handled in startup.tsx useEffect now) Co-authored-by: Jan Six <[email protected]> * remove commented out code Co-authored-by: Jan Six <[email protected]> * replace px values with tokens Co-authored-by: Jan Six <[email protected]> * conditional export for AsyncMessageChannel preview env * add browser preview WEBSOCKETS_PORT env * fix typescript issue with PreviewAsyncMessageChannel.isWsConnected * add test coverage for AsyncMessageChannelPreview * Browser preview debug UI (#2803) * fix AsyncMessageChannelPreview undefined error + export WS URI * browser preview CSS file for UI fixes * create previewUtils for browser color scheme + setFigmaBrowserTheme * browser preview URL params + fullscreen/theme/action modes * two bug fixes for browser/plugin websocket preview bridge * add preview dist folder for web preview builds * [WIP] browser preview dev knowledge docs * feat(dev): request startup on browser preview page open * refactor(dev): use env vars for browser preview ws src * fix(debug): remove console.log from asyncmessagechannelpreview * fix(css): figmaloading full height css for browser preview * refactor(dev): use enums for websockets src in browser preview tsx * fix(dev): remove comments * refactor: reuse htmlClassList variable * remove unused package --------- Co-authored-by: macintoshhelper <[email protected]> --------- Co-authored-by: macintoshhelper <[email protected]> Co-authored-by: Jan Six <[email protected]>
* Browser plugin preview (#2748) * [WIP] webpack config setup for fast refresh + websocket server * add prod/dev hmr webpack config option * render immediately and handle AsyncMessage in startup app hook * forward ui AsyncMessages to browser via WebSockets * null check sentry transaction to fix browser error * refactor AsyncMessageChannel code for browser implementation * webpack use swc-loader for browser version + speed/bundle size plugin options * prettify browser dev preview UI * enable loading screen if startup params missing (for web serve + disconnected browser dev preview) * attempt to fix webpack build for tests * add SpeedMeasurePlugin package * create AsyncMessageChannel dev docs * replace web-preview.md ASCII data flow diagram with mermaid * use radii/spacing tokens instead of px for web preview.tsx styles Co-authored-by: Jan Six <[email protected]> * remove commented out startup handler (handled in startup.tsx useEffect now) Co-authored-by: Jan Six <[email protected]> * remove commented out code Co-authored-by: Jan Six <[email protected]> * replace px values with tokens Co-authored-by: Jan Six <[email protected]> * conditional export for AsyncMessageChannel preview env * add browser preview WEBSOCKETS_PORT env * fix typescript issue with PreviewAsyncMessageChannel.isWsConnected * add test coverage for AsyncMessageChannelPreview * Browser preview debug UI (#2803) * fix AsyncMessageChannelPreview undefined error + export WS URI * browser preview CSS file for UI fixes * create previewUtils for browser color scheme + setFigmaBrowserTheme * browser preview URL params + fullscreen/theme/action modes * two bug fixes for browser/plugin websocket preview bridge * add preview dist folder for web preview builds * [WIP] browser preview dev knowledge docs * feat(dev): request startup on browser preview page open * refactor(dev): use env vars for browser preview ws src * fix(debug): remove console.log from asyncmessagechannelpreview * fix(css): figmaloading full height css for browser preview * refactor(dev): use enums for websockets src in browser preview tsx * fix(dev): remove comments * refactor: reuse htmlClassList variable * remove unused package --------- Co-authored-by: macintoshhelper <[email protected]> --------- Co-authored-by: macintoshhelper <[email protected]> Co-authored-by: Jan Six <[email protected]> * Browser plugin preview (#2748) * [WIP] webpack config setup for fast refresh + websocket server * add prod/dev hmr webpack config option * render immediately and handle AsyncMessage in startup app hook * forward ui AsyncMessages to browser via WebSockets * null check sentry transaction to fix browser error * refactor AsyncMessageChannel code for browser implementation * webpack use swc-loader for browser version + speed/bundle size plugin options * prettify browser dev preview UI * enable loading screen if startup params missing (for web serve + disconnected browser dev preview) * attempt to fix webpack build for tests * add SpeedMeasurePlugin package * create AsyncMessageChannel dev docs * replace web-preview.md ASCII data flow diagram with mermaid * use radii/spacing tokens instead of px for web preview.tsx styles Co-authored-by: Jan Six <[email protected]> * remove commented out startup handler (handled in startup.tsx useEffect now) Co-authored-by: Jan Six <[email protected]> * remove commented out code Co-authored-by: Jan Six <[email protected]> * replace px values with tokens Co-authored-by: Jan Six <[email protected]> * conditional export for AsyncMessageChannel preview env * add browser preview WEBSOCKETS_PORT env * fix typescript issue with PreviewAsyncMessageChannel.isWsConnected * add test coverage for AsyncMessageChannelPreview * Browser preview debug UI (#2803) * fix AsyncMessageChannelPreview undefined error + export WS URI * browser preview CSS file for UI fixes * create previewUtils for browser color scheme + setFigmaBrowserTheme * browser preview URL params + fullscreen/theme/action modes * two bug fixes for browser/plugin websocket preview bridge * add preview dist folder for web preview builds * [WIP] browser preview dev knowledge docs * feat(dev): request startup on browser preview page open * refactor(dev): use env vars for browser preview ws src * fix(debug): remove console.log from asyncmessagechannelpreview * fix(css): figmaloading full height css for browser preview * refactor(dev): use enums for websockets src in browser preview tsx * fix(dev): remove comments * refactor: reuse htmlClassList variable * remove unused package --------- Co-authored-by: macintoshhelper <[email protected]> --------- Co-authored-by: macintoshhelper <[email protected]> Co-authored-by: Jan Six <[email protected]> * update web preview script docs * fix: updateStyles error from merge conflict * lint: remove comments and cleanup * fix: language switch on startup for new preview startup * lint: remove more comments from preview files * lint: fix indentation error because of prettier conflict --------- Co-authored-by: macintoshhelper <[email protected]> Co-authored-by: Jan Six <[email protected]>
Why does this PR exist?
Closes #2709
There is now an optional web preview mode for development, which starts an Express.js server locally with a WebSockets server. The plugin UI forwards messages between the controller and the browser version.
What does this pull request do?
npm run preview:browser
+npm run preview:plugin
npm run build
+npm run serve
workingSTARTUP
handler is moved touseEffect
in the startup component.yarn build
+yarn serve
yarn preview:browser
yarn preview:plugin
Testing this change
Start WebSockets server and
webpack-dev-server
for web preview and HMRStart Figma plugin dev
Additional Notes (if any)
May need to reload the plugin in Figma if
STARTUP
runs too early. Am thinking to implement a sort of dev SDK preview window in the browser, with the ability to simulateSTARTUP
with custom data, would need something like this to work in a Next.js preview and handle the case of having no WebSocket server or Figma plugin environment runningThe
preview:browser
script is running an Express server with a WebSockets server, sonpm run serve
isn't working yet.Plugin UI is disabled when
PREVIEW_MODE
/browser mode is active.Currently Fast Refresh requires stopping the plugin preview in the command line, to prevent
STARTUP
from restarting the browser plugin UI.Unrelated change, but I added dev npm packages for
BundleAnalyzerPlugin
andSpeedMeasurePlugin
to make debugging build time/bundle size easier when needed (pass Webpack arg). These affect build times so are disabled by default.