Figma-plugin-refactor#269
Conversation
- apply folder structure changes
- change folder structure
- add prefix(Figma,Extract)
Walkthrough이 PR은 Figma 플러그인의 노드 변환 파이프라인을 재구성하며, 타입 정의, 추출 유틸리티, 변수 바인딩 시스템을 추가합니다. 또한 여러 컴포넌트 Storybook 문서와 디자인 규칙을 신규 생성합니다. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 분 변환 파이프라인 재구성, 다양한 유틸리티 함수, 광범위한 타입 정의, 수많은 Storybook 문서 추가로 인해 검토 범위가 크고 이질적입니다. 특히 노드 변환 로직, 변수 바인딩 시스템, 기존 코드 삭제 부분의 검증이 필요합니다. Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Deploy preview for re-creation ready! ✅ Preview Built with commit 02c7b7d. |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shared/figma-plugin/src/server.ts (1)
167-189:get_selection도구가 하드코딩된 "test"를 반환합니다.
sendCommandToFigma("get_selection")호출 결과를 무시하고 하드코딩된"test"문자열을 반환하고 있습니다. 이것이 디버그 코드라면 실제 결과를 반환하도록 수정해야 합니다.🔧 실제 결과를 반환하도록 수정
async () => { try { const result = await sendCommandToFigma("get_selection") return { content: [ { type: "text", - text: "test", + text: JSON.stringify(result), }, ], }
🤖 Fix all issues with AI agents
In `@shared/figma-plugin/rules/question.stories.mdx`:
- Around line 240-248: The story object LastRequiredQuestion has a typo in its
args.title string ("최소 하나의 문제은 필요합니다")—update args.title to "최소 하나의 문제는 필요합니다"
so the Korean grammar is correct; locate the LastRequiredQuestion export and
edit the title field accordingly.
In `@shared/figma-plugin/src/convert/node/instance.ts`:
- Around line 20-22: 현재 instance.ts에서 extractNodeStyle(node)와 collectCss(node)
사이에 남아있는 디버그 출력(console.log(node.componentProperties))이 플러그인 런타임에 민감한 디자인 데이터를
노출하고 로그 노이즈를 증가시킵니다; 해당 console.log 호출을 삭제하거나(권장) 환경/디버그 플래그로 제어되는 로거로 대체하도록
수정하고, 참조 대상인 extractNodeStyle, collectCss, node.componentProperties 및 문제의
console.log 호출을 찾아 제거 또는 안전한 로깅으로 변경하세요.
In `@shared/figma-plugin/src/figmaClient.ts`:
- Around line 64-79: The current response-matching condition ignores responses
that have errors or falsy results because it requires myResponse.result to be
truthy; change the outer if to only check myResponse.id and
pendingRequests.has(myResponse.id) so every response with a matching id is
handled. Inside that block keep clearing the timeout and retrieving the request
(pendingRequests.get(myResponse.id)), then if myResponse.error call logger.error
and request.reject(new Error(...)), else call request.resolve(myResponse.result)
even when result is falsy, and finally pendingRequests.delete(myResponse.id).
- Around line 43-52: The connectToFigma function currently only checks
ws.readyState === WebSocket.OPEN before returning, allowing a new WebSocket to
be created when ws is in WebSocket.CONNECTING; update connectToFigma to treat
both OPEN and CONNECTING as "already connecting/connected" and return early
(i.e., check ws && (ws.readyState === WebSocket.OPEN || ws.readyState ===
WebSocket.CONNECTING)) to prevent duplicate socket creation and resource leaks
while leaving existing ws variable and logging intact.
In `@shared/figma-plugin/tsup.config.js`:
- Around line 5-16: The loadRules function currently reads every entry in
rulesDir and blindly calls readFileSync, which can throw on subdirectories or
non-.mdx files and produce non-deterministic output; update loadRules to: list
entries from rulesDir, filter to regular files only (use fs.lstatSync or
fs.statSync + isFile()) and filter by extension (e.g., path.extname(file) ===
".mdx"), sort the filtered file list (e.g., files.sort()) for deterministic
order, then call readFileSync for each remaining file and populate the rules
object; reference symbols: loadRules, rulesDir, files, readFileSync,
path.extname, fs.lstatSync/fs.statSync.
🧹 Nitpick comments (11)
package.json (1)
37-41: dependenciesMeta 버전 고정 범위 확인이 필요합니다.
현재 키가@modelcontextprotocol/sdk@1.24.3로 고정되어 있어 실제 해석 버전이 다르면 unplugged가 적용되지 않을 수 있습니다. 해석 버전을 확인한 뒤, 필요하면 패키지명만 지정해 범위를 넓히는 쪽이 안전합니다.🔧 제안 변경
- "dependenciesMeta": { - "@modelcontextprotocol/sdk@1.24.3": { - "unplugged": true - } - } + "dependenciesMeta": { + "@modelcontextprotocol/sdk": { + "unplugged": true + } + }shared/figma-plugin/src/convert/boundVariables.ts (1)
3-4: 불필요한 래퍼 함수 제거 권장
withBoundVariables는collectNodeVariableReferences를 그대로 호출하는 것 외에 추가 로직이 없습니다. 직접 호출하는 것이 더 간결합니다.♻️ 간소화 제안
import { collectNodeVariableReferences } from "../utils/variables/variableCollector" -const withBoundVariables = (node: SceneNode) => - collectNodeVariableReferences(node) - export const ensureBoundVariables = ( node: SceneNode, ): ReturnType<typeof collectNodeVariableReferences> | undefined => { - const references = withBoundVariables(node) + const references = collectNodeVariableReferences(node) return references.length > 0 ? references : undefined }shared/figma-plugin/src/styles.css (1)
1-5::root선언이 중복되어 있습니다.
:root블록이 두 번 정의되어 있습니다 (1-5줄, 38-51줄). 단일:root블록으로 통합하는 것이 유지보수에 좋습니다.♻️ 통합 제안
:root { font-family: Inter, system-ui, Helvetica, Arial, sans-serif; - font-display: optional; font-size: 16px; -} - -/* ... other styles ... */ - -:root { --radius-full: 9999px; --radius-large: 0.8125rem; --radius-medium: 0.3125rem; --radius-none: 0; --radius-small: 0.125rem; --spacer-0: 0; --spacer-1: 0.25rem; --spacer-2: 0.5rem; --spacer-3: 1rem; --spacer-4: 1.5rem; --spacer-5: 2rem; --spacer-6: 2.5rem; }참고:
font-display: optional은 일반적으로@font-face규칙 내에서 사용됩니다.:root에서의 사용이 의도된 것인지 확인해 주세요.Also applies to: 38-51
shared/figma-plugin/rules/icon.stories.mdx (1)
16-29: 아이콘 네이밍 컨벤션을 확인해주세요.대부분의 아이콘은 PascalCase(
Add,Arrow,Cross)를 따르지만, placeholder 아이콘들은 underscore를 사용합니다(Iconplaceholder_16px). 일관성을 위해IconPlaceholder16px같은 형식으로 통일하는 것이 좋을 수 있습니다.shared/figma-plugin/src/utils/serialize/convertNodeToXML.ts (3)
3-8: 불필요한async선언입니다.이 함수는
await를 사용하지 않으므로 동기 함수로 변경할 수 있습니다.js2xml은 동기 함수입니다.♻️ 권장 수정안
-export async function convertNodeToXML( +export function convertNodeToXML( selectedNodes: readonly SceneNode[], -): Promise<string> { +): string {
10-16:any타입 사용을 줄이는 것을 고려해주세요.
processNode의 반환 타입과attributes객체에any가 사용되고 있습니다. xml-js의Element타입이나 더 구체적인 타입을 사용하면 타입 안전성이 향상됩니다.♻️ 타입 개선 제안
+import type { Element } from "xml-js" + +type XmlAttributes = Record<string, string | number | boolean> - const processNode = (node: SceneNode): any => { + const processNode = (node: SceneNode): Element | null => { if (!node) { return null } - const attributes: any = {} + const attributes: XmlAttributes = {}
119-124: COMPONENT와 INSTANCE 타입 제외 이유를 문서화해주세요.
COMPONENT와INSTANCE타입의 children을 제외하는 이유가 명확하지 않습니다. 주석을 추가하면 향후 유지보수에 도움이 됩니다.📝 주석 추가 제안
+ // COMPONENT와 INSTANCE 타입은 children을 별도로 처리하거나 + // 재귀적으로 포함하지 않아야 하는 경우가 있어 제외합니다 const children = "children" in node && node.type !== "COMPONENT" && node.type !== "INSTANCE" ? node.children.map(processNode).filter((child) => child !== null) : []shared/design/convertStoryToMdx.ts (2)
156-158: 모듈 임포트 시 자동 실행 방지를 고려해주세요.
main()이 무조건 실행되어 이 모듈을 다른 곳에서 임포트하면 의도치 않게 실행됩니다. 직접 실행 여부를 체크하는 것이 좋습니다.♻️ 권장 수정안
-// Run if invoked directly - -main() +// Run if invoked directly +const isMain = + import.meta.url === `file://${process.argv[1]}` || + process.argv[1]?.endsWith("convertStoryToMdx.ts") + +if (isMain) { + main() +}또는 ES 모듈에서 더 간단하게:
+if (import.meta.url.endsWith(process.argv[1]?.split(/[\\/]/).pop() ?? "")) { + main() +}
78-85:fileExists함수가 모든 에러를 무시합니다.권한 문제나 다른 파일 시스템 에러도
false로 반환됩니다.ENOENT에러만 처리하는 것이 더 안전합니다.♻️ 에러 처리 개선안
async function fileExists(p: string) { try { await fs.stat(p) return true - } catch { + } catch (err) { + if ((err as NodeJS.ErrnoException).code === "ENOENT") { + return false + } + throw err - return false } }shared/figma-plugin/src/utils/mappings/map.ts (1)
200-211: 일관성을 위해 누락된 타입 별칭 추가를 고려하세요.
STROKE_CAP_TO_CSS와STROKE_JOIN_TO_CSS매핑에 대한 타입 별칭(FigmaStrokeCap,FigmaStrokeJoin)이 정의되어 있지만, 다른 매핑들과 달리 export되지 않았습니다. 또한FigmaLineHeightUnit과FigmaLetterSpacingUnit타입도 누락되어 있습니다.♻️ 누락된 타입 별칭 추가
export type FigmaBlendMode = keyof typeof BLEND_MODE_TO_CSS export type FigmaStrokeAlign = keyof typeof STROKE_ALIGN_TO_CSS +export type FigmaStrokeCap = keyof typeof STROKE_CAP_TO_CSS +export type FigmaStrokeJoin = keyof typeof STROKE_JOIN_TO_CSS +export type FigmaLineHeightUnit = keyof typeof LINE_HEIGHT_UNIT_TO_CSS +export type FigmaLetterSpacingUnit = keyof typeof LETTER_SPACING_UNIT_TO_CSSshared/figma-plugin/src/convert/index.ts (1)
24-29: COMPONENT/COMPONENT_SET 캐스팅 대신 타입 확장 권장
FrameNode로 강제 캐스팅하면 타입 불일치가 가려질 수 있습니다.buildFrameNode시그니처를 확장해 캐스팅을 제거하는 편이 안전합니다.♻️ 제안 수정
- baseNode = await buildFrameNode(node as FrameNode) + baseNode = await buildFrameNode(node)// shared/figma-plugin/src/convert/node/frame.ts -export const buildFrameNode = async ( - node: FrameNode, -): Promise<FrameReactNode> => { +export const buildFrameNode = async ( + node: FrameNode | ComponentNode | ComponentSetNode, +): Promise<FrameReactNode> => {
| export const LastRequiredQuestion: Story = { | ||
| args: { | ||
| state: "default", | ||
| index: 1, | ||
| title: "최소 하나의 문제은 필요합니다", | ||
| canDelete: false, | ||
| }, | ||
| render: Template, | ||
| } |
There was a problem hiding this comment.
오타 수정이 필요합니다.
Line 244에서 "문제은"은 "문제는"으로 수정해야 합니다.
✏️ 수정안
- title: "최소 하나의 문제은 필요합니다",
+ title: "최소 하나의 문제는 필요합니다",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const LastRequiredQuestion: Story = { | |
| args: { | |
| state: "default", | |
| index: 1, | |
| title: "최소 하나의 문제은 필요합니다", | |
| canDelete: false, | |
| }, | |
| render: Template, | |
| } | |
| export const LastRequiredQuestion: Story = { | |
| args: { | |
| state: "default", | |
| index: 1, | |
| title: "최소 하나의 문제는 필요합니다", | |
| canDelete: false, | |
| }, | |
| render: Template, | |
| } |
🤖 Prompt for AI Agents
In `@shared/figma-plugin/rules/question.stories.mdx` around lines 240 - 248, The
story object LastRequiredQuestion has a typo in its args.title string ("최소 하나의
문제은 필요합니다")—update args.title to "최소 하나의 문제는 필요합니다" so the Korean grammar is
correct; locate the LastRequiredQuestion export and edit the title field
accordingly.
| const style = extractNodeStyle(node) | ||
| console.log(node.componentProperties) | ||
| const css = await collectCss(node) |
There was a problem hiding this comment.
디버그 로그 제거 권장
플러그인 런타임 로그는 디자인 데이터 노출/노이즈의 원인이 될 수 있습니다.
🧹 제안 수정
- console.log(node.componentProperties)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const style = extractNodeStyle(node) | |
| console.log(node.componentProperties) | |
| const css = await collectCss(node) | |
| const style = extractNodeStyle(node) | |
| const css = await collectCss(node) |
🤖 Prompt for AI Agents
In `@shared/figma-plugin/src/convert/node/instance.ts` around lines 20 - 22, 현재
instance.ts에서 extractNodeStyle(node)와 collectCss(node) 사이에 남아있는 디버그
출력(console.log(node.componentProperties))이 플러그인 런타임에 민감한 디자인 데이터를 노출하고 로그 노이즈를
증가시킵니다; 해당 console.log 호출을 삭제하거나(권장) 환경/디버그 플래그로 제어되는 로거로 대체하도록 수정하고, 참조 대상인
extractNodeStyle, collectCss, node.componentProperties 및 문제의 console.log 호출을 찾아
제거 또는 안전한 로깅으로 변경하세요.
| function connectToFigma(port = 3055) { | ||
| if (ws && ws.readyState === WebSocket.OPEN) { | ||
| logger.info("Already connected to Figma") | ||
| return | ||
| } | ||
|
|
||
| const wsUrl = serverUrl === "localhost" ? `${wsUrlBase}:${port}` : wsUrlBase | ||
| logger.info(`Connecting to Figma socket server at ${wsUrl}...`) | ||
| ws = new WebSocket(wsUrl) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the full figmaClient.ts file to understand context
cat shared/figma-plugin/src/figmaClient.tsRepository: JECT-Study/JECT-5-FE
Length of output: 184
🌐 Web query:
WebSocket readyState CONNECTING OPEN values JavaScript
💡 Result:
In JavaScript WebSocket.readyState is one of four numeric constants:
- WebSocket.CONNECTING = 0 — connection not yet open.
- WebSocket.OPEN = 1 — connection open and ready to communicate.
- WebSocket.CLOSING = 2 — connection is closing.
- WebSocket.CLOSED = 3 — connection closed or failed to open.
Example: if (socket.readyState === WebSocket.OPEN) { /* send */ }. [1][2]
Sources:
[1] MDN WebSocket.readyState.
[2] WebSocket API reference.
연결 중(CONNECTING) 상태에서도 새 소켓을 열 수 있어 리소스 누수가 발생합니다.
ws.readyState 값이 WebSocket.CONNECTING (0)일 때도 새 소켓이 생성되므로, 중복 연결 시도를 방지하려면 OPEN 상태뿐만 아니라 CONNECTING 상태도 함께 확인해야 합니다.
수정 제안
- if (ws && ws.readyState === WebSocket.OPEN) {
- logger.info("Already connected to Figma")
- return
- }
+ if (
+ ws &&
+ (ws.readyState === WebSocket.OPEN ||
+ ws.readyState === WebSocket.CONNECTING)
+ ) {
+ logger.info("Already connected or connecting to Figma")
+ return
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function connectToFigma(port = 3055) { | |
| if (ws && ws.readyState === WebSocket.OPEN) { | |
| logger.info("Already connected to Figma") | |
| return | |
| } | |
| const wsUrl = serverUrl === "localhost" ? `${wsUrlBase}:${port}` : wsUrlBase | |
| logger.info(`Connecting to Figma socket server at ${wsUrl}...`) | |
| ws = new WebSocket(wsUrl) | |
| function connectToFigma(port = 3055) { | |
| if ( | |
| ws && | |
| (ws.readyState === WebSocket.OPEN || | |
| ws.readyState === WebSocket.CONNECTING) | |
| ) { | |
| logger.info("Already connected or connecting to Figma") | |
| return | |
| } | |
| const wsUrl = serverUrl === "localhost" ? `${wsUrlBase}:${port}` : wsUrlBase | |
| logger.info(`Connecting to Figma socket server at ${wsUrl}...`) | |
| ws = new WebSocket(wsUrl) |
🤖 Prompt for AI Agents
In `@shared/figma-plugin/src/figmaClient.ts` around lines 43 - 52, The
connectToFigma function currently only checks ws.readyState === WebSocket.OPEN
before returning, allowing a new WebSocket to be created when ws is in
WebSocket.CONNECTING; update connectToFigma to treat both OPEN and CONNECTING as
"already connecting/connected" and return early (i.e., check ws &&
(ws.readyState === WebSocket.OPEN || ws.readyState === WebSocket.CONNECTING)) to
prevent duplicate socket creation and resource leaks while leaving existing ws
variable and logging intact.
| if ( | ||
| myResponse.id && | ||
| pendingRequests.has(myResponse.id) && | ||
| myResponse.result | ||
| ) { | ||
| const request = pendingRequests.get(myResponse.id)! | ||
| clearTimeout(request.timeout) | ||
|
|
||
| if (myResponse.error) { | ||
| logger.error(`Error from Figma: ${myResponse.error}`) | ||
| request.reject(new Error(myResponse.error)) | ||
| } else if (myResponse.result) { | ||
| request.resolve(myResponse.result) | ||
| } | ||
|
|
||
| pendingRequests.delete(myResponse.id) |
There was a problem hiding this comment.
응답 매칭 조건 때문에 에러/빈 결과가 타임아웃됩니다.
현재 myResponse.result가 truthy일 때만 pending 요청을 처리해서, 에러 응답이나 0/false/"" 같은 결과가 무시됩니다. 결과적으로 타임아웃으로만 종료될 수 있으니 id 매칭만으로 처리하도록 수정이 필요합니다.
🐛 수정 제안
- if (
- myResponse.id &&
- pendingRequests.has(myResponse.id) &&
- myResponse.result
- ) {
+ if (myResponse.id && pendingRequests.has(myResponse.id)) {
const request = pendingRequests.get(myResponse.id)!
clearTimeout(request.timeout)
if (myResponse.error) {
logger.error(`Error from Figma: ${myResponse.error}`)
request.reject(new Error(myResponse.error))
- } else if (myResponse.result) {
- request.resolve(myResponse.result)
- }
+ } else {
+ request.resolve(myResponse.result)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ( | |
| myResponse.id && | |
| pendingRequests.has(myResponse.id) && | |
| myResponse.result | |
| ) { | |
| const request = pendingRequests.get(myResponse.id)! | |
| clearTimeout(request.timeout) | |
| if (myResponse.error) { | |
| logger.error(`Error from Figma: ${myResponse.error}`) | |
| request.reject(new Error(myResponse.error)) | |
| } else if (myResponse.result) { | |
| request.resolve(myResponse.result) | |
| } | |
| pendingRequests.delete(myResponse.id) | |
| if (myResponse.id && pendingRequests.has(myResponse.id)) { | |
| const request = pendingRequests.get(myResponse.id)! | |
| clearTimeout(request.timeout) | |
| if (myResponse.error) { | |
| logger.error(`Error from Figma: ${myResponse.error}`) | |
| request.reject(new Error(myResponse.error)) | |
| } else { | |
| request.resolve(myResponse.result) | |
| } | |
| pendingRequests.delete(myResponse.id) |
🤖 Prompt for AI Agents
In `@shared/figma-plugin/src/figmaClient.ts` around lines 64 - 79, The current
response-matching condition ignores responses that have errors or falsy results
because it requires myResponse.result to be truthy; change the outer if to only
check myResponse.id and pendingRequests.has(myResponse.id) so every response
with a matching id is handled. Inside that block keep clearing the timeout and
retrieving the request (pendingRequests.get(myResponse.id)), then if
myResponse.error call logger.error and request.reject(new Error(...)), else call
request.resolve(myResponse.result) even when result is falsy, and finally
pendingRequests.delete(myResponse.id).
| // 빌드 시점에 rules 파일들을 읽어서 객체로 만듦 | ||
| function loadRules() { | ||
| const rulesDir = path.join(process.cwd(), "rules") | ||
| const files = fs.readdirSync(rulesDir) | ||
| const rules = {} | ||
|
|
||
| for (const file of files) { | ||
| const content = fs.readFileSync(path.join(rulesDir, file), "utf-8") | ||
| rules[file] = content | ||
| } | ||
|
|
||
| return rules |
There was a problem hiding this comment.
rules 로딩에서 디렉터리/비‑MDX 파일 처리 누락은 빌드 실패로 이어질 수 있습니다.
규칙 폴더에 하위 폴더나 다른 파일이 생기면 readFileSync가 예외를 던질 수 있고, 파일 순서도 OS에 따라 달라져 빌드 산출물이 비결정적일 수 있습니다. 파일 필터링과 정렬을 추가하는 편이 안전합니다.
🛠️ 제안 변경
function loadRules() {
const rulesDir = path.join(process.cwd(), "rules")
- const files = fs.readdirSync(rulesDir)
+ const files = fs
+ .readdirSync(rulesDir, { withFileTypes: true })
+ .filter((entry) => entry.isFile())
+ .map((entry) => entry.name)
+ .filter((name) => name.endsWith(".mdx"))
+ .sort()
const rules = {}
for (const file of files) {
const content = fs.readFileSync(path.join(rulesDir, file), "utf-8")
rules[file] = content
}🤖 Prompt for AI Agents
In `@shared/figma-plugin/tsup.config.js` around lines 5 - 16, The loadRules
function currently reads every entry in rulesDir and blindly calls readFileSync,
which can throw on subdirectories or non-.mdx files and produce
non-deterministic output; update loadRules to: list entries from rulesDir,
filter to regular files only (use fs.lstatSync or fs.statSync + isFile()) and
filter by extension (e.g., path.extname(file) === ".mdx"), sort the filtered
file list (e.g., files.sort()) for deterministic order, then call readFileSync
for each remaining file and populate the rules object; reference symbols:
loadRules, rulesDir, files, readFileSync, path.extname,
fs.lstatSync/fs.statSync.
📝 설명
figma plugin 일부 기능 변경
Summary by CodeRabbit
릴리스 노트
새로운 기능
스타일
Chores
✏️ Tip: You can customize this high-level summary in your review settings.