Skip to content

chore: upgrade jsdoc to latest to support node 23+#2009

Closed
HerrTopi wants to merge 1 commit into
masterfrom
update-jsdoc
Closed

chore: upgrade jsdoc to latest to support node 23+#2009
HerrTopi wants to merge 1 commit into
masterfrom
update-jsdoc

Conversation

@HerrTopi

@HerrTopi HerrTopi commented Jun 3, 2025

Copy link
Copy Markdown
Contributor

TEST_PLAN: check if everything works in docs with latest(24.x) and older(20.x) node

INSTUI-4574

@HerrTopi HerrTopi requested review from balzss and matyasf June 3, 2025 18:21
@github-actions

github-actions Bot commented Jun 3, 2025

Copy link
Copy Markdown
PR Preview Action v1.6.1

🚀 View preview at
https://instructure.design/pr-preview/pr-2009/

Built to branch gh-pages at 2025-06-04 08:42 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@HerrTopi HerrTopi self-assigned this Jun 4, 2025
Comment thread package.json
]
],
"dependencies": {
"clipboardy": "^4.0.0"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

cant this be a dev dep?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

or is this even needed?

@matyasf matyasf left a comment

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.

Looks OK, but see my comment about adding a comment. I'm not 100% convinced that this is the right way to do it, but it gets the job done for now

Comment on lines +62 to +70
onListening: async function () {
// devServer is watching source files by default and hot reloading the docs page if they are changed
// however markdown files (i.e. README.md) need to be recompiled hence the need for chokidar
const paths = globbySync(['packages/**/*.md', 'docs/**/*.md'], { cwd: '../../' }).map(p => '../../' + p)
chokidar
.watch(paths)
.on('change', (evt) => {
.on('change', async (evt) => {
const fullPath = resolvePath(import.meta.dirname, evt)
processSingleFile(fullPath)
await processSingleFile(fullPath)

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.

Both the Webpack onListening https://webpack.js.org/configuration/dev-server/#devserveronlistening
and Chokidar's on https://github.com/paulmillr/chokidar are not async.

I think this should work fine for 99% of use cases (unless you somehow modify the same file in very quick succession?) but please put a comment to both functions, that these do not wait for the promise to return.

@HerrTopi HerrTopi assigned matyasf and unassigned HerrTopi Jul 3, 2025
@HerrTopi HerrTopi closed this Jul 3, 2025
@HerrTopi

HerrTopi commented Jul 3, 2025

Copy link
Copy Markdown
Contributor Author

Dulicate

@matyasf matyasf deleted the update-jsdoc branch July 15, 2025 20:43
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.

3 participants