Skip to content

Update frontend onStart guidance for simple startup code#3182

Merged
nighca merged 1 commit into
goplus:devfrom
nighca:issue-3176
May 19, 2026
Merged

Update frontend onStart guidance for simple startup code#3182
nighca merged 1 commit into
goplus:devfrom
nighca:issue-3176

Conversation

@nighca
Copy link
Copy Markdown
Collaborator

@nighca nighca commented May 19, 2026

Summary

Update frontend-facing onStart guidance so simple startup code no longer needs to default to an explicit onStart callback, while keeping onStart documented for the cases where it is still the right tool.

Changes

  • update the spx-project skill to allow direct top-level startup statements for simple cases
  • remove the outdated guidance that the first var block cannot assign values
  • clarify onStart API/reference text as an explicit startup callback that does not block other event callbacks
  • refresh related examples in the SPX skill references and Copilot code-change example

Validation

  • npx eslint src/components/editor/spx-code-editor/document-base/index.ts src/components/editor/copilot/CodeChange.vue

Closes #3176

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the SPX documentation and examples to allow and encourage the use of top-level sequential statements for simple startup logic, rather than strictly requiring onStart. It also clarifies that onStart should be used for non-blocking or multi-handler scenarios. Feedback suggests further clarifying the synchronous nature of top-level statements to warn users about potential blocking of the event loop.

Comment thread spx-gui/src/utils/spx/skills/spx-project/SKILL.md Outdated
Copy link
Copy Markdown
Contributor

@fennoai fennoai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall the direction is correct: allowing simple top-level startup statements reduces boilerplate and aligns with how children naturally write code. A few accuracy and consistency issues are worth addressing before merging.

Comment thread spx-gui/src/utils/spx/skills/spx-project/SKILL.md
Comment thread spx-gui/src/utils/spx/skills/spx-project/SKILL.md
Comment thread spx-gui/src/components/editor/spx-code-editor/document-base/index.ts Outdated
Comment thread spx-gui/src/utils/spx/skills/spx-project/references/apis.md Outdated
println word
```

ALSO RIGHT:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

没太看懂,RIGHT / ALSO RIGHT 看起来没什么关系?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这块内容是说明怎么组织初始化代码(以及变量定义、事件监听逻辑)是合适的,RIGHT 是初始化代码直接写在 top level,ALSO RIGHT 是初始化代码用 onStart 包起来的写法,这俩种写法都可能是合理的

@nighca nighca merged commit 3536bbf into goplus:dev May 19, 2026
4 checks passed
qingqing-ux added a commit that referenced this pull request May 19, 2026
* Update UI empty/error default images (#3134)

* update ui empty/error default images

* compress svg size

* rename image

* adjust svg size

* chore(deps): bump github.com/goplus/xgolsw to 0.20.0 in /tools/spxls (#3163)

Signed-off-by: Aofei Sheng <aofei@aofeisheng.com>

* feat(spx-gui): optimize video cut preview during animation generation (#3150)

* optimize video cut preview during animation generation

* use css variables for track padding

* fix details

* remove page for test

* optimize code

* queue video seeks to keep animation preview responsive during dragging

* chore(deps): bump spx to 2.0.1 (#3162)

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: nighca <1492263+nighca@users.noreply.github.com>

* Add empty `ui/prototype/` to avoid Vercel deployment failure (#3164)

* Add empty ui/prototype/ to avoid Vercel deployment failure

* Update ui/prototype/package.json

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

---------

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* feat(animation): migrate sound binding from onStart to onPlay (#3168)

* feat(animation): migrate sound binding from onStart to onPlay

- Export animation sound binding using `onPlay` instead of `onStart`
  to align with the animation lifecycle semantics in spx v2.0.0
- Load sound binding from `onPlay` as the primary field; fall back to
  legacy `onStart` for backward compatibility with existing projects
- Update `RawAnimationConfig`: `onPlay` is now the current API field,
  `onStart` is moved to the legacy section
- Add tests for the new export behavior and legacy onStart loading

Fixes #3116

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: nighca <1492263+nighca@users.noreply.github.com>

* fix(animation): add missing blank line between test cases

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: nighca <1492263+nighca@users.noreply.github.com>

* feat(animation): add loop field to ActionConfig type

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: nighca <1492263+nighca@users.noreply.github.com>

* feat(animation): add soundLoop field with default false, parse loop from raw config

- Add soundLoop boolean field (default false) to AnimationInits and Animation class
- Add setSoundLoop() method to Animation
- Parse loop from onPlay/onStart in load() for backward compatibility
- Export loop: true in onPlay only when soundLoop is true (omitting it preserves old behavior)
- Preserve soundLoop in clone()
- Add tests for load/export/clone of soundLoop field

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: nighca <1492263+nighca@users.noreply.github.com>

* fix(animation): always export loop field in onPlay, including loop: false

Export loop: false when soundLoop is false so the exported config is
explicit. "default false" only applies during loading when loop is absent
from the raw config.

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: nighca <1492263+nighca@users.noreply.github.com>

* fix(animation): remove onStart.loop handling, no legacy loop field

onStart is a legacy field predating the loop feature; old configs using
onStart never contain loop. Only read loop from onPlay.

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: nighca <1492263+nighca@users.noreply.github.com>

* fix(animation): only set soundLoop when sound is valid

soundLoop is now set inside the else branch so it is only assigned
when the sound lookup succeeds and soundId is set.

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: nighca <1492263+nighca@users.noreply.github.com>

* fix(animation): assert soundLoop is false in legacy onStart load test

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: nighca <1492263+nighca@users.noreply.github.com>

* fix(animation): remove duplicate loop export test case

'should export loop: false in onPlay when soundLoop is false (default)'
is fully covered by 'should export sound binding using onPlay' which
already asserts { play: ..., loop: false }.

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: nighca <1492263+nighca@users.noreply.github.com>

---------

Co-authored-by: xgopilot <noreply@goplus.org>
Co-authored-by: nighca <1492263+nighca@users.noreply.github.com>

* fix(animation): revert sound binding to onStart temporarily (#3174)

* fix(animation): revert sound binding to onStart temporarily

* test(animation): document temporary skipped onPlay cases

* docs(animation): clarify temporary onStart export comment

* Sync SPX overload documentation mappings (#3175)

* Sync SPX overload documentation mappings

* Address review feedback

* Handle fennoai review feedback

* refactor: update descriptions and initialization guidelines for onStart (#3182)

* fix: increase maximum file size limit to 64MB (#3184)

* docs(openapi): make product APIs resource-oriented (#3146)

Replace the existing product API paths with the new resource-oriented
contract. Move authenticated-user collections under `/user/*`, public
user collections under `/users/{username}/*`, and relationship
membership operations to idempotent `PUT` routes.

Add operation IDs, specific path parameter names, and `Location`
headers for created resources while preserving moved-resource response
semantics for renamed resources.

Updates #3144

Signed-off-by: Aofei Sheng <aofei@aofeisheng.com>

---------

Signed-off-by: Aofei Sheng <aofei@aofeisheng.com>
Co-authored-by: chennan <chennan@qiniu.com>
Co-authored-by: Aofei Sheng <aofei@aofeisheng.com>
Co-authored-by: Hanxing Yang <nighca@live.cn>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: nighca <1492263+nighca@users.noreply.github.com>
Co-authored-by: fennoai[bot] <231223108+fennoai[bot]@users.noreply.github.com>
Co-authored-by: xgopilot <noreply@goplus.org>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: wyvern <wuxinyi@qiniu.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update frontend prompts, skills, and API docs for omitted onStart

2 participants