docs: api history#42982
docs: api history#42982VerteDinde merged 30 commits intoelectron:mainfrom piotrpdev:feat/api-history
Conversation
This reverts commit 0a11efc.
dsanders11
left a comment
There was a problem hiding this comment.
Left a few comments, but overall looking good.
Co-authored-by: David Sanders <dsanders11@ucsbalum.com>
Co-authored-by: Erick Zhao <erick@hotmail.ca> Reference: electron/website@0b1b6a7
dsanders11
left a comment
There was a problem hiding this comment.
@electron/wg-api, soliciting your opinions on a few subjective bits here regarding where to place API history blocks for some of the broader changes.
- For #40330 there's not a direct API to place the API history under. Currently Piotr has opted to put it under module-level changes for both
contextBridgeandipcRenderer. We've also discussed if it would make sense to place it under additional methods inipcRenderer/contextBridge. Thoughts on what balance to strike regarding over documenting in too many places versus being comprehensive? - For the
BrowserViewdeprecation it's currently placed as API history under both the module and the class.
* feat(api-history): basic webpack and remark plugin * feat(api-history): basic table * feat(api-history): minor table styling * refactor(api-history): move regexp to variable * feat(api-history): better regexp * feat(api-history): move plugin to own file * feat(api-history): remove `removed` property * feat(api-history): fetch version using pr * feat(api-history): types for data fetch code * refactor(api-history): more readable row sorting * feat(api-history): changes markdown support * feat(api-history): strip comment tags in pre-build instead of plugin * feat(api-history): `pr-release-versions-plugin` * feat(api-history): use react component instead of handmade AST * fix(api-history): revert accidental dependency * refactor(api-history): get pr releases in transformer, remove plugin * docs(api-history): todo in transformer * docs(api-history): add temp example api history and schema for pr review * fix(api-history): update `yarn.lock` for now * feat(api-history): use example pr versions by default for now #594 (comment) * refactor(api-history): move `ApiHistoryTable` styles to module #594 (comment) * style(api-history): better var name, fix style lint #594 (comment) * feat(api-history): map pre-releases to released stable versions * feat(api-history): better styles, remove pr number * feat(api-history): link table change to breaking-changes * fix(api-history): provide key for table elements * feat(api-history): table semver ranges Co-authored-by: Samuel Attard <sam@electronjs.org> * feat(api-history): better error handling #594 (comment) * feat(api-history): type predicates in transformer #594 (comment) * feat(api-history): update to accepted schema electron/rfcs#6 * docs(api-history): remove old comments * feat(api-history): only pre-process `/docs/api/*.md` containing `<!--` * Update scripts/tasks/preprocess-api-history.ts Co-authored-by: Kevin Cui <158blackhole@gmail.com> * fix(api-history): match api history block consistently * feat(api-history): pre-process more error checking * Update docs/latest/api-history.schema.json Co-authored-by: Erick Zhao <erick@hotmail.ca> * fix(api-history): duplicate dependencies * docs(api-history): remove unnecessary comment * refactor(api-history): move pre-process deps to dev deps * feat(api-history): `isHtml(node)` type guard #594 (comment) * fix(api-history): missing `toLowerCase()` * Update scripts/tasks/preprocess-api-history.ts Co-authored-by: Kevin Cui <158blackhole@gmail.com> * Update scripts/tasks/preprocess-api-history.ts Co-authored-by: Kevin Cui <158blackhole@gmail.com> * Update src/components/ApiHistoryTable.tsx Co-authored-by: Kevin Cui <158blackhole@gmail.com> * revert(api-history): `isHtml(node)` type guard This reverts commit 9481b1c. * feat(api-history): tell user to run `lint-roller` * fix(api-history): stricter change type param * Apply suggestions from code review Co-authored-by: David Sanders <dsanders11@ucsbalum.com> * style(api-history): fix lint * fix(api-history): remove unnecessary null check * style(api-history): fix lint * docs(api-history): stricter schema Reference: electron/electron@c9de2c5 * docs(api-history): `ipc-renderer` Reference: electron/electron#42982 (comment) * docs(api-history): `browser-view` * ci: use GH token with correct permissions * fix(api-history): change semver sign if backport is `x.0.0` Reported-by: David Sanders <dsanders11@ucsbalum.com> * fix(api-history): preprocess warn on missing schema * Update src/components/ApiHistoryTable.tsx Co-authored-by: David Sanders <dsanders11@ucsbalum.com> * refactor(api-history): better semver logic Co-authored-by: David Sanders <dsanders11@ucsbalum.com> Based-on-patch-by: David Sanders <dsanders11@ucsbalum.com> Reference: #594 (comment) * refactor(api-history): remove test `docs/` `git diff feat/api-history origin/main -- docs/latest/api/ | git apply` * refactor(api-history): remove test PR data --------- Co-authored-by: Samuel Attard <sam@electronjs.org> Co-authored-by: Kevin Cui <158blackhole@gmail.com> Co-authored-by: Erick Zhao <erick@hotmail.ca> Co-authored-by: David Sanders <dsanders11@ucsbalum.com>
itsananderson
left a comment
There was a problem hiding this comment.
Those approaches sound reasonable to me.
The one thing I'm noticing is that there seems to be duplication of the breaking-changes-header values. I assume that gets used for an HTML ID for deep linking, but if we're using duplicate IDs won't that make deep linking not work properly?
| ```YAML history | ||
| deprecated: | ||
| - pr-url: https://github.com/electron/electron/pull/35658 | ||
| breaking-changes-header: deprecated-browserview |
There was a problem hiding this comment.
Do we need to ensure that each of the breaking-changes-header values are unique? Otherwise I assume it'll only be possible to link to the top-most item.
There was a problem hiding this comment.
Both GitHub and Docusaurus will automatically de-duplicate heading slugs by appending a numeral to the end: https://gist.github.com/erickzhao/808369ac0f08d1d26adbc066eeb9c4ec
I think this actually raises a good point since we add more recent breaking changes to the top of the doc. In the instance that we actually change an API in a breaking way multiple times, then the header slugs are no longer stable.
e.g.
API changed: session.setPermissionCheckHandler(handler) in Electron 13 currently has the #api-changed-sessionsetpermissioncheckhandlerhandler slug.
If we make another modification/breaking change to the same API in Electron 40, the initial heading slug will change to #api-changed-sessionsetpermissioncheckhandlerhandler-1.
Ensuring headings are unique sounds like the cleanest way to implement this approach IMO and provides a better experience for the breaking changes doc anyways.
VerteDinde
left a comment
There was a problem hiding this comment.
Ready to merge for the 32 stable release 👍
|
No Release Notes |
|
I was unable to backport this PR to "32-x-y" cleanly; |
|
@VerteDinde has manually backported this PR to "32-x-y", please check out #43362 |
Description of Change
Note
This PR is part of the 2024 Electron GSoC [Project] [Proposal].
Note
You can see what API History will look like on the website in electron/website#594
Depends on #42950.
Adds API History to the documentation, this includes:
lint:api-historyinpackage.jsonlint:api-historyalso appended tolint:docsdocs/styleguide.mddocs/development/api-history-migration-guide.mdNote
Non-removed breaking changes from
breaking-changes.mdsince>=25.0.0.https://gist.github.com/piotrpdev/5abe5514374567756e71fc2f07457e56
clearHistory,canGoBack,goBack,canGoForward,goForward,canGoToOffset,goToOffsetonWebContentsnativeImage.toDataURLwill preserve PNG colorspacewindow.flashFrame(bool)will flash dock icon continuously on macOSBehavior Changed: cross-origin iframes now use Permission Policy to access featuresBrowserView.setAutoResizebehavior on macOSipcRenderercan no longer be sent over thecontextBridgeWebContents.backgroundThrottlingset to false affects allWebContentsin the hostBrowserWindowprotocol.{register,intercept}{Buffer,String,Stream,File,Http}Protocol@dsanders11 @erickzhao @VerteDinde
Checklist
npm testpassesRelease Notes
Notes: none