Conversation
- remove requirement for hardcoded index.md file - optionally limit the header level reported `auto_toc: 2` will report only level 1 and level 2 headers in the files - scan and report folder contents (not recursively yet).
📝 WalkthroughWalkthroughThis PR reorganizes the Module Development docs: adds many new pages (Connection Basics/Advanced, Module Setup, Module Lifecycle), updates category metadata and links, enhances TOC generation and markdown styling, and includes numerous how‑to guides and examples for module authors. Changes
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (22)
for-developers/module-development/connection-basics/presets.md-8-10 (1)
8-10:⚠️ Potential issue | 🟡 MinorSmall typo: missing word "need" 🙂
Line 10 reads "you will to call" — looks like "need" slipped away!
Suggested fix
-In order to add presets to a module, you will to call another function in your module. The process is similar to how you define actions and feedbacks. +To add presets to a module, you will need to call another function in your module. The process is similar to how you define actions and feedbacks.for-developers/module-development/connection-advanced/setting-custom-variables.md-1-15 (1)
1-15:⚠️ Potential issue | 🟡 MinorMissing YAML front matter — this might cause unexpected behavior in Docusaurus
Every other doc in this PR includes front matter (
title,sidebar_label,sidebar_position,description). Without it, Docusaurus will try to infer the title from the first content line (the blockquote warning), which probably isn't what you want. Would you mind adding front matter here too? Something like:Suggested addition
+--- +title: Setting Custom Variables +sidebar_label: Custom Variables +description: How to output data to user-owned custom variables from an action. +--- + > This is an experimental idea, that may be removed without noticefor-developers/module-development/connection-basics/upgrade-scripts.md-9-19 (1)
9-19:⚠️ Potential issue | 🟡 MinorA few small text and lint nits throughout the file 📝
A handful of minor things the linter caught, plus a grammar one:
- Line 9: "a on/off" → "an on/off"
- Line 13: "practise" → "practice" (more standard in tech documentation)
- Line 19: The fenced code block is missing a language identifier (e.g.,
```js). Flagged by markdownlint (MD040).Suggested fixes
-A common example is changing a on/off option to be an on/off/toggle option. +A common example is changing an on/off option to be an on/off/toggle option.-Each upgrade script will only get run once for each action and feedback, but it is good practise to write the scripts so that they can be executed multiple times. This will help you when testing your script, or if jumping between versions of companion. +Each upgrade script will only get run once for each action and feedback, but it is good practice to write the scripts so that they can be executed multiple times. This will help you when testing your script, or if jumping between versions of companion.-``` +```js const upgradeScripts = require('./upgrades')for-developers/module-development/connection-basics/upgrade-scripts.md-83-85 (1)
83-85:⚠️ Potential issue | 🟡 MinorSmall grammar nit on the last line 🙂
"Anything changes" → "Any changes"
Suggested fix
##### return result -This tells Companion what had changed in your script. Anything changes made to actions or feedbacks not listed in the result object will be discarded. +This tells Companion what has changed in your script. Any changes made to actions or feedbacks not listed in the result object will be discarded.for-developers/module-development/connection-basics/user-configuration.md-5-5 (1)
5-5:⚠️ Potential issue | 🟡 MinorDescription doesn't match the page content — looks copy-pasted from feedback docs 📋
The
descriptionfield says "Module feedback definition details" but this page is about user-config definitions. This text often shows up in search results and meta tags, so it'd be nice to fix it.Suggested fix
-description: Module feedback definition details. +description: Module user-config definition details.for-developers/module-development/connection-advanced/subscribe-unsubscribe-flow.md-44-44 (1)
44-44:⚠️ Potential issue | 🟡 MinorMissing apostrophes: "isnt" → "isn't", "arent" → "aren't"
Suggested fix
-This optimisation isnt necessary for most modules, but if your users end up with hundreds of feedbacks of the same type, this will help ensure there arent performance issues. +This optimisation isn't necessary for most modules, but if your users end up with hundreds of feedbacks of the same type, this will help ensure there aren't performance issues.for-developers/module-development/connection-basics/variables.md-10-10 (1)
10-10:⚠️ Potential issue | 🟡 MinorSmall typo — "change update" looks like a leftover edit 🙂
Looks like there might be a stray word here. Maybe it should read "define variables and update their values"?
Suggested fix
-This section explains how to define variables and change update their values. +This section explains how to define variables and update their values.for-developers/module-development/connection-advanced/migrating-legacy-to-boolean-feedbacks.md-9-9 (1)
9-9:⚠️ Potential issue | 🟡 MinorTypo: "trigges" → "triggers", and missing period
Suggested fix
-A benefit of this, is that these boolean feedbacks can be used as conditions in the trigges system. We hope to allow for more complex behaviours with these feedbacks, but before that is worth doing we need more modules to be utilising the new type of feedback +A benefit of this is that these boolean feedbacks can be used as conditions in the triggers system. We hope to allow for more complex behaviours with these feedbacks, but before that is worth doing we need more modules to be utilising the new type of feedback.for-developers/module-development/connection-basics/user-configuration.md-25-25 (1)
25-25:⚠️ Potential issue | 🟡 MinorMissing space between sentences
There's a missing space between "...more details." and "The linked documentation...".
Suggested fix
-The fields you can use here are similar to the ones for actions and feedbacks, but with more limitations. See the [list of field types](./input-field-types.md) for more details.The linked documentation states any limitations that apply when used for the configuration, or if they are not allowed. +The fields you can use here are similar to the ones for actions and feedbacks, but with more limitations. See the [list of field types](./input-field-types.md) for more details. The linked documentation states any limitations that apply when used for the configuration, or if they are not allowed.for-developers/module-development/connection-advanced/migrating-legacy-to-boolean-feedbacks.md-138-140 (1)
138-140:⚠️ Potential issue | 🟡 MinorStep numbering jumps from 2 to 4, and small typo 📋
The steps go
### 1,### 2, then### 4— step 3 is missing! Either there's content that hasn't been added yet, or the numbering just needs adjusting.Also on line 140: "help with self" → "help with this".
Suggested fix
-### 4. Add an upgrade script +### 3. Add an upgrade script -Users will have feedbacks assigned to buttons already, and these will all need updating to the new format. A helper has been added to help with self. +Users will have feedbacks assigned to buttons already, and these will all need updating to the new format. A helper has been added to help with this.for-developers/module-development/connection-advanced/subscribe-unsubscribe-flow.md-37-37 (1)
37-37:⚠️ Potential issue | 🟡 MinorMissing opening backtick breaks inline code formatting
this.unsubscribeActions()is missing its opening backtick, so it'll render as plain text with a stray backtick.Suggested fix
-For actions, the methods `this.subscribeActions()` and this.unsubscribeActions()` can be used instead +For actions, the methods `this.subscribeActions()` and `this.unsubscribeActions()` can be used instead.for-developers/module-development/connection-basics/feedbacks.md-148-148 (1)
148-148:⚠️ Potential issue | 🟡 MinorTODO marker visible to readers
_TODO the linked page doesn't say much?_This note would be visible in the published docs. It might be worth removing it or converting it to an HTML comment (
<!-- TODO ... -->) so it's hidden from readers.for-developers/module-development/connection-basics/actions.md-99-99 (1)
99-99:⚠️ Potential issue | 🟡 MinorBroken link:
../api-changes/companion-4.1.mddoesn't resolve 🔗Hey there! The build pipeline is flagging that this link can't be resolved. It looks like the
api-changes/companion-4.1.mdfile might not exist yet in this PR. Would you mind either adding that file, updating the link to point to the correct location, or perhaps leaving just the version reference without the link for now?for-developers/module-development/connection-basics/input-field-types.md-77-77 (1)
77-77:⚠️ Potential issue | 🟡 MinorTODO marker visible to readers
_TODO: write an advanced page for this? or add to user-config?_Same note as the other files — this will show up in published docs. Consider hiding it in an HTML comment or removing it before merge.
for-developers/module-development/connection-basics/feedbacks.md-110-110 (1)
110-110:⚠️ Potential issue | 🟡 MinorBroken link:
../api-changes/companion-4.1.mddoesn't resolveSame issue as in
actions.md— the pipeline flags this link as unresolvable. Likely the target file hasn't been added to this PR yet.for-developers/module-development/connection-basics/actions.md-105-107 (1)
105-107:⚠️ Potential issue | 🟡 MinorTODO marker left in published documentation 📝
Just a heads-up — this TODO is visible to readers:
TODO: Was the following true for actions or only feedbacks?
Totally fine to have TODOs during development, but it'd be great to resolve this before merging so readers don't see unfinished notes. If you're not sure of the answer yet, maybe wrap the uncertain paragraph in a
:::cautionadmonition or remove it for now?Would you like me to open an issue to track this TODO so it doesn't get lost?
for-developers/module-development/connection-basics/feedbacks.md-65-70 (1)
65-70:⚠️ Potential issue | 🟡 MinorMissing comma in JS example — syntax error 🐛
There's a missing comma after the
labelvalue on line 67. This would be a syntax error if someone copies this example directly.Suggested fix
options: [{ type: 'number', - label: 'Source' + label: 'Source', id: 'source', default: 1 }],for-developers/module-development/connection-basics/feedbacks.md-87-87 (1)
87-87:⚠️ Potential issue | 🟡 MinorTypo: missing space in "documentationlinked"
Small typo — "documentationlinked" should be "documentation linked".
Suggested fix
-There are more properties available in feedback definitions, which are described in full in the documentationlinked in the [previous section](`#feedback-types`). +There are more properties available in feedback definitions, which are described in full in the documentation linked in the [previous section](`#feedback-types`).for-developers/module-development/connection-basics/input-field-types.md-22-24 (1)
22-24:⚠️ Potential issue | 🟡 MinorMissing comma in JS example — syntax error
Same issue as in
feedbacks.md: missing comma after thelabelstring on line 23.Suggested fix
{ type: 'number', - label: 'Source' + label: 'Source', id: 'source',for-developers/module-development/connection-basics/feedbacks.md-22-22 (1)
22-22:⚠️ Potential issue | 🟡 MinorCopy-paste slip: "actions" should be "feedbacks" ✏️
Looks like this sentence was borrowed from the actions page — it says "split the actions definitions" but since we're in the feedbacks doc, it should say "feedbacks definitions" (or "feedback definitions").
Suggested fix
-The [TypeScript module template](https://github.com/bitfocus/companion-module-template-ts) includes a file `src/feedbacks.ts` which is where your feedbacks should be defined. It is not required to use this structure, but it keeps it more readable than having everything in one file. More complex modules will likely want to split the actions definitions into even more files/folders. +The [TypeScript module template](https://github.com/bitfocus/companion-module-template-ts) includes a file `src/feedbacks.ts` which is where your feedbacks should be defined. It is not required to use this structure, but it keeps it more readable than having everything in one file. More complex modules will likely want to split the feedback definitions into even more files/folders.for-developers/module-development/connection-basics/input-field-types.md-13-13 (1)
13-13:⚠️ Potential issue | 🟡 MinorIncomplete sentence: "When defining an ," ✂️
It looks like a word got lost here! The sentence reads "When defining an ," which is missing its noun. Perhaps it should be something like "When defining an action, feedback, or config"?
Suggested fix
-When defining an , the actions, feedbacks and module config definition object includes a property called `options:` that takes a list of input-field definitions. For example, +When defining an action, feedback, or module config, the definition object includes a property called `options:` that takes a list of input-field definitions. For example,for-developers/module-development/connection-advanced/bonjour-device-discovery.md-1-3 (1)
1-3:⚠️ Potential issue | 🟡 MinorMissing YAML front matter 📋
All the other new docs in this PR include front matter (
---block with title, sidebar_label, sidebar_position, description). This file is missing it, which could affect how Docusaurus renders the page title, sidebar entry, and meta description. Would you mind adding something like:+--- +title: Bonjour Device Discovery +sidebar_label: Bonjour Discovery +description: How to use Bonjour (MDNS) for device discovery in Companion modules. +--- + Bonjour is a standardised method of device discovery, utilising MDNS.
🧹 Nitpick comments (10)
for-developers/module-development/index.md (1)
1-7: Small suggestion on the description — totally optional!
"Outline Module Development"reads a bit like a verb phrase rather than a description. Something like"An overview of module development for Companion"might flow more naturally, but no big deal either way. 🙂for-developers/module-development/connection-basics/presets.md (3)
12-12: Heading level jump flagged by markdownlint (MD001)The first heading after the doc title (h1) is
###(h3). Markdown best practice is to increment heading levels one at a time, so this should be##. The same applies to the other###headings (lines 49, 59, 113) — they'd naturally become##as well. Totally up to you whether to address now or later!
55-55: Link text "here" isn't very descriptive (MD059)Screen readers and accessibility tools benefit from meaningful link text. A small tweak would help:
Suggested fix
-You can see the full list of values that can be set and their valid values in the `style` object [here](https://bitfocus.github.io/companion-module-base/interfaces/CompanionButtonStyleProps.html) +You can see the full list of values that can be set and their valid values in the [`style` object documentation](https://bitfocus.github.io/companion-module-base/interfaces/CompanionButtonStyleProps.html)
70-89: Minor: inconsistent indentation in the code exampleLine 75 (
rotate_left) has an extra leading space compared to the surrounding properties (down,up,rotate_right). Just a tiny formatting nit — no rush!src/remark/autoTocPlugin.mjs (2)
96-98:startsWithis slightly too loose for filename exclusion — consider===insteadUsing
startsWith(vfilename)means if your current file is e.g.index.md, it would also exclude a hypotheticalindex.md.bakor similar. A strict equality check would be more precise and avoids surprises.Suggested fix
- const files = readdirSync(dir).filter((f) => f.endsWith('.md') && !f.startsWith(vfilename)) + const files = readdirSync(dir).filter((f) => f.endsWith('.md') && f !== vfilename)
186-200: Subdirectory rendering uses baretextandbreaknodes at the document root — may render unpredictably 🌲The tree-drawing characters (├──, └──) with braille blanks for indentation are a creative approach! Just a heads-up though: bare
textandbreaknodes outside of aparagraphin mdast aren't standard, and different renderers (or future Docusaurus versions) may handle them inconsistently. Screen readers could also announce the braille characters (U+2800) unexpectedly.If you run into rendering quirks, wrapping each sub-file entry in a
paragraphnode (or using a nested list) would be more robust. No action required right now if it's rendering fine for you — just flagging it as something to be aware of!for-developers/module-development/connection-basics/actions.md (1)
91-93: Consider more descriptive link text for accessibilityThe markdownlint tool flags generic link text like "here" (MD059). More descriptive text helps screen readers and makes scanning easier. For example:
Suggested improvement
-There are more properties available, which are described in full in [the autogenerated Actions documentation on GitHub](https://bitfocus.github.io/companion-module-base/interfaces/CompanionActionDefinition.html) +There are more properties available, which are described in full in [the autogenerated Actions documentation on GitHub](https://bitfocus.github.io/companion-module-base/interfaces/CompanionActionDefinition.html). -The `options` property of the action definition is an array of input types, as defined [here](./input-field-types.md) +The `options` property of the action definition is an array of input types, as defined in [Input Field Types](./input-field-types.md).for-developers/module-development/connection-advanced/bonjour-device-discovery.md (1)
38-56: Code block missing language specifierThis fenced code block doesn't have a language tag, so it won't get syntax highlighting. Adding
jswould be consistent with the other code blocks in this file and across the PR.Suggested fix
-``` +```js { type: 'textinput',for-developers/module-development/connection-basics/feedbacks.md (2)
93-93: Heading level skips from h2 to h4The markdownlint tool flags this (MD001): heading levels should increment by one. Since the parent is
##, this should be###instead of####.Suggested fix
-#### Inverting boolean feedbacks +### Inverting boolean feedbacks
99-104: Code block missing language specifierAdding a language tag (e.g.,
js) would enable syntax highlighting and be consistent with the rest of the doc.Suggested fix
-``` +```js CreateUseBuiltinInvertForFeedbacksUpgradeScript({
for-developers/module-development/connection-advanced/learn-action-feedback-values.md
Outdated
Show resolved
Hide resolved
for-developers/module-development/connection-advanced/migrating-legacy-to-boolean-feedbacks.md
Outdated
Show resolved
Hide resolved
for-developers/module-development/connection-advanced/migrating-legacy-to-boolean-feedbacks.md
Show resolved
Hide resolved
for-developers/module-development/connection-advanced/migrating-legacy-to-boolean-feedbacks.md
Outdated
Show resolved
Hide resolved
for-developers/module-development/connection-advanced/subscribe-unsubscribe-flow.md
Outdated
Show resolved
Hide resolved
for-developers/module-development/connection-basics/upgrade-scripts.md
Outdated
Show resolved
Hide resolved
|
I'm not keen on that http://localhost:4005/for-developers/module-development page. I feel like this would be better as a handcrafted page. For http://localhost:4005/for-developers/module-development/connection-basics/, I wonder if this auto-toc is beneficial, vs the other category layout. Because the headings only really make sense within the context of the page they are from. |
There seems to be alot of overlap between module-development-101.md, your new overview.md, and user-configuration.md I realized we're missing two big issues: the module class itself, and establishing connections. So I added stuff. Copy-edited: - actions.md - overview.md Added: - connecting.md Updated: - variables.md - presets.md (typos caught something in v2.0.md)
I hear your argument, especially wrt the API log needing the auto-toc more than other sections, but think it's still better than the silly cards. Maybe we can refine later? At worst, we can remove it from this PR if we can't agree too easily.
Not inconsistent! Just to be sure we're on the same page: they indicate whether it's an in-page link vs. a file in a subdir. "├──" is pretty standard notation for "text folder-trees" Put a bit more extravagantly: It's like saying this message is inconsistent because some lines start with a vertical line, some with a bullet and some are flush left! (I suppose you could say I'm inconsistent in not showing headings for files in the subdirs, but that's the point at which I start thinking "its quite a wall of links.") That said, we can certainly debate styling or whether or how much should be there at all. My thought is that the current design provides a little more depth/overview than the sidebar, which starts with all categories closed.
Not sure I understand this concern: isn't the context right there in the auto-toc as well? I think in general, its value is more in giving an overview of the section-structure than providing in-page links, though both can be useful... FWIW, I'd just as soon not do anything hand-crafted because these pages are likely-as-not to be overlooked -- you only get it if you click the name of the group as opposed to clicking on the caret... OK, stepping off for today. All of my local changes have been pushed to here. |
- minor fix to autToc - transfer bonjour.png from a "hidden" asset - fix typos (feedbacks.md)
build now works but you may have to run `yarn docusaurus clear` to clear the build caches (deleting /build is likely not enough).
|
Important note from the commit comment:
done for (my) today. |
plus minor fixes
…s' into chore_transfer_main_module_docs
- CSS: make H2 and H3 smaller -- more like the default ifm. (Docusaurus makes them much too big and with too much spacing) - fix two autotoc bugs -- quotes were shown if a title had quotes in it -- clicking on the link to an index worked, but then all links in that file were broken.
This completes the transfer of all files from the wiki note: I also put module-debugging at the top level of modules
surprisingly most of its comments were still valid!
With the CSS change, the text size is unchanged, so this is more consistent (previously there was no H2, only H1 and H3 in the files).
|
@coderabbitai do not flag "unhidden" TODO comments separately -- they are useful to let the reader know that something may be out of date. (It's ok to have a single comment listing all TODOs). |
|
I'll kick off the review now! ✏️ Learnings added
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 4
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
for-developers/module-development/api-changes/v1.8.md (1)
14-18:⚠️ Potential issue | 🟡 MinorTiny grammar nit in the tip block — remove the comma and clarify the closing sentence.
The comma after "API" on line 16 creates an awkward pseudo-imperative reading, and "those" in the last sentence is a bit ambiguous. A small tweak would make it much cleaner:
✏️ Suggested wording
-Newer versions of the API, replace this with newer ways of achieving the same result. Only follow this if you can't update to those +Newer versions of the API replace this with newer ways of achieving the same result. Only follow this section if you cannot update to a newer API version.for-developers/module-development/api-changes/v2.0.md (2)
27-27:⚠️ Potential issue | 🟡 MinorTiny typo in the package name heading:
@companion-mdoule/base→@companion-module/baseEasy to miss! The section heading on line 27 has
mdouleinstead ofmodule. Worth fixing while we're touching this file so developers don't copy the wrong package name.✏️ Suggested fix
-### `@companion-mdoule/base` is now ESM +### `@companion-module/base` is now ESM
255-255:⚠️ Potential issue | 🟡 MinorUnclosed backtick in heading may break rendering
The heading reads
### Referencing expressions from `isVisibleExpression— the inline-code backtick is never closed, which most Markdown renderers will propagate as a runaway code span, potentially mangling everything that follows until the next backtick.✏️ Suggested fix
-### Referencing expressions from `isVisibleExpression +### Referencing expressions from `isVisibleExpression`for-developers/module-development/api-changes/v1.13.md (1)
52-58:⚠️ Potential issue | 🟡 MinorA few typos in the new
subscribe/unsubscribesection 📝
- Line 52 (heading):
"Action/"→"Actions/"(missing the plurals)- Line 56:
"noisey"→"noisy"- Line 58:
"subscrube"→"subscribe"✏️ Suggested fixes
-## subscribe/unsubscribe: More flexibility for Action/ less for feedbacks {`#subscribe`} +## subscribe/unsubscribe: More flexibility for Actions / less for feedbacks {`#subscribe`}-They can also specify which options should not trigger a call to subscribe/unsubscribe, allowing you to filter out noisey/excessive calls. +They can also specify which options should not trigger a call to subscribe/unsubscribe, allowing you to filter out noisy/excessive calls.-Feedback subscrube/unsubscribe callbacks are no longer called for every options change +Feedback subscribe/unsubscribe callbacks are no longer called for every options changefor-developers/module-development/home.md (1)
64-64:⚠️ Potential issue | 🟡 MinorSingle-item numbered list looks incomplete.
The
Install the dependenciessection has1. yarn installas the only step. If this is intentionally a single instruction, a plain bullet- \yarn install`reads better. If more steps (e.g.,yarn build`) were intended, they seem to have been left out.
🟡 Minor comments (45)
for-developers/module-development/api-changes/v1.8.md-25-25 (1)
25-25:⚠️ Potential issue | 🟡 MinorSmall grammar nip: "suggest them" is missing an indirect object.
"suggest them whilst they type" reads a little oddly — "them" needs a recipient. Something like "suggest these to users as they type" flows more naturally.
✏️ Suggested wording
-This allows us to show a hint to the user about this support, and suggest them whilst they type. +This allows us to show a hint to the user about this support, and suggest these variables to them as they type.for-developers/linking-to-companion.md-21-21 (1)
21-21:⚠️ Potential issue | 🟡 MinorTiny pre-existing nit: double "not" on line 21.
"you may not not change its color"— onenottoo many! This is untouched by the current PR, but since you're already editing nearby, it'd be an easy fix to sneak in if you'd like. Totally up to you 😊✏️ Suggested fix
-You can scale the image to fit in your website, as long as it is still readable, you may not not change its color, rotation, animate it or change it in any other way than scaling. +You can scale the image to fit in your website, as long as it is still readable, you may not change its color, rotation, animate it or change it in any other way than scaling.for-developers/module-development/connection-basics/connecting.md-107-107 (1)
107-107:⚠️ Potential issue | 🟡 MinorTypo in the TelnetHelper link text — "Telent" → "Telnet"
The URL is already correct (
TelnetHelperEvents.html), but the visible link text reads "TelentHelperEvents" — just two transposed letters to swap. 😊✏️ Proposed fix
-- [TelentHelperEvents](https://bitfocus.github.io/companion-module-base/interfaces/TelnetHelperEvents.html) +- [TelnetHelperEvents](https://bitfocus.github.io/companion-module-base/interfaces/TelnetHelperEvents.html)for-developers/module-development/connection-basics/connecting.md-8-19 (1)
8-19:⚠️ Potential issue | 🟡 MinorA few small wording/grammar nits in the intro 📝
Nothing blocking, just a handful of easy polish items:
- Line 8: "One of the most first tasks" → "One of the first tasks" (drop the redundant "most").
- Line 12: "builtin" → "built-in"; "fetch api" → "fetch API" (standard capitalisation for the Web API name).
- Line 19: "possible TCP event --" → "possible TCP events --" (plural, since several events are listed right below).
✏️ Proposed fixes
-One of the most first tasks most modules have to perform is to connect to their device. +One of the first tasks most modules have to perform is to connect to their device.-Or if the device uses HTTP, we recommend using the builtin [`fetch` api](https://nodejs.org/en/learn/getting-started/fetch). +Or if the device uses HTTP, we recommend using the built-in [`fetch` API](https://nodejs.org/en/learn/getting-started/fetch).-You define the various `on()` callbacks to respond to possible TCP event -- see the example, below. +You define the various `on()` callbacks to respond to possible TCP events — see the example below.for-developers/module-development/connection-basics/connecting.md-37-77 (1)
37-77:⚠️ Potential issue | 🟡 MinorTwo syntax errors in the TCPHelper code example — worth fixing before this goes live! 🐛
Great example overall — two small bugs crept in that would trip up anyone copy-pasting it:
- Line 50 — mismatched quotes: the string opens with
'but closes with".- Line 70 — the
tcp.on('status_change', ...)call is missing its closing).🐛 Proposed fixes
tcp.on('connect', () => { this.updateStatus(InstanceStatus.Ok) - this.log('debug', 'Connected!") + this.log('debug', 'Connected!') })tcp.on('status_change', (state, message) => { this.updateStatus(state, message) this.log('debug', 'Status Changed to ' + state + (message != undefined ? ': ' + message : '')) - } + })for-developers/module-development/module-debugging.md-8-9 (1)
8-9:⚠️ Potential issue | 🟡 MinorTiny typo: "will be come" → "will become"
✏️ Proposed fix
-Once you've started coding, debugging the code will be come essential to your success. +Once you've started coding, debugging the code will become essential to your success.for-developers/module-development/module-debugging.md-59-63 (1)
59-63:⚠️ Potential issue | 🟡 MinorDouble-"not" typo in the warning callout
"may not not possible" is missing "be" and has an extra "not".
✏️ Proposed fix
-It may not not possible to debug the `init` method from your module with this… +It may not be possible to debug the `init` method from your module with this…for-developers/module-development/module-debugging.md-17-17 (1)
17-17:⚠️ Potential issue | 🟡 MinorTwo minor grammar nits flagged by the linter
- Line 17: "module specific" → "module-specific" (compound modifier before a noun should be hyphenated)
- Line 57: "builtin" → "built-in"
✏️ Proposed fixes
-You can open the module specific log view from the connections page by clicking… +You can open the module-specific log view from the connections page by clicking…-You can use any compatible debugger such as the builtin VS Code debugger… +You can use any compatible debugger such as the built-in VS Code debugger…for-developers/module-development/connection-advanced/oauth.md-16-16 (1)
16-16:⚠️ Potential issue | 🟡 MinorMissing period at end of sentence
"…as support is improved" is missing a closing
.✏️ Suggested fix
-Described below is the current recommended way of supporting OAuth, but many existing modules are not using this. Expect this to be refined in the future, as support is improved +Described below is the current recommended way of supporting OAuth, but many existing modules are not using this. Expect this to be refined in the future, as support is improved.for-developers/module-development/connection-advanced/http-handler.md-8-8 (1)
8-8:⚠️ Potential issue | 🟡 MinorTypo: missing possessive apostrophe — "Companions" → "Companion's"
✏️ Suggested fix
-Companions webserver that serves the Web UI also provides a path for each connection... +Companion's webserver that serves the Web UI also provides a path for each connection...for-developers/module-development/connection-advanced/oauth.md-26-26 (1)
26-26:⚠️ Potential issue | 🟡 MinorBroken markdown link —
(HTTP handler)isn't a valid URLThe link target
HTTP handler(a plain string with a space) won't resolve. Based on the sibling file, the correct relative path should be./http-handler.md.🔗 Suggested fix
-To receive the authentication code, you should implement the [handleHttpRequest](HTTP handler) method, listening for the `/oauth/callback` path. +To receive the authentication code, you should implement the [handleHttpRequest](./http-handler.md) method, listening for the `/oauth/callback` path.for-developers/module-development/connection-advanced/oauth.md-47-56 (1)
47-56:⚠️ Potential issue | 🟡 Minor
responseis used but never declared — sample code will throw aReferenceErrorAfter the
// TODOplaceholder, lines 54–55 referenceresponse.data.accessToken/response.data.refreshTokenwithoutresponseever being assigned. Anyone copying this snippet as-is will hit a runtime error. Even as example code, adding a commented placeholder variable makes the intent clear and prevents confusion.✏️ Suggested clarification
try { // Exchange code for token - // TODO: Implement your logic here + // TODO: Replace with your actual token-exchange HTTP call, e.g.: + // const response = await axios.post('https://...', { code: authCode, ... }) //Save new values to Configurationsrc/remark/autoTocPlugin.mjs-99-100 (1)
99-100:⚠️ Potential issue | 🟡 Minor
startsWithmay accidentally exclude sibling filesHey, nice work on this! One thing I noticed — on line 99,
!f.startsWith(vfilename)would also exclude files that happen to start with the same prefix. For example, if the current file isindex.md, a sibling namedindex-extra.mdwould be filtered out too. An exact-match check (f !== vfilename) seems safer here.Also a small optimization opportunity:
readdirSync(dir)is called twice (lines 99 and 100). You could stash the result once and filter from it.Suggested tweak
- const files = readdirSync(dir).filter((f) => f.endsWith('.md') && !f.startsWith(vfilename)) - const subdirs = readdirSync(dir).filter((f) => existsSync(join(dir, f, '_category_.json'))) + const dirEntries = readdirSync(dir) + const files = dirEntries.filter((f) => f.endsWith('.md') && f !== vfilename) + const subdirs = dirEntries.filter((f) => existsSync(join(dir, f, '_category_.json')))src/css/custom.css-272-285 (1)
272-285:⚠️ Potential issue | 🟡 MinorHeading typography overrides look well-reasoned — minor Stylelint formatting nit
Love the detailed comment block documenting the defaults! The sizing choices make sense for documentation readability.
Stylelint flags lines 274 and 280 for
declaration-empty-line-before— it expects an empty line between custom property declarations and regular declarations within the same rule block:Formatting fix to satisfy Stylelint
.markdown h1 { --ifm-h1-vertical-rhythm-bottom: 1; + font-size: 2em; /* setting --ifm-h1-font-size doesn't work (because of specificity?) */ } .markdown h2 { --ifm-h2-vertical-rhythm-top: 1; /* default 2 is too much. Also, we already indent everything under H2 to make H2 breaks clear */ --ifm-heading-vertical-rhythm-bottom: calc(0.5 / 1.25); /* this is multiplied by --ifm-leading = 1.25 */ + font-size: 1.5em; }src/remark/autoTocPlugin.mjs-126-128 (1)
126-128:⚠️ Potential issue | 🟡 MinorSame
startsWithconcern here for excluding the index fileSimilar to the sibling-file filter above — if
indexfileis"overview", thenf.startsWith("overview")also excludes"overview-advanced.md". An equality or slug-based comparison would be more precise:Suggested tweak
- const subfiles = readdirSync(subdir).filter((f) => f.endsWith('.md') && !f.startsWith(indexfile)) + const subfiles = readdirSync(subdir).filter( + (f) => f.endsWith('.md') && f !== indexfile && f !== `${indexfile}.md` + )This handles both the case where
indexfilealready includes.md(the default'index.md') and where it's just the doc id (e.g.,"overview").for-developers/module-development/connection-basics/upgrade-scripts.md-79-79 (1)
79-79:⚠️ Potential issue | 🟡 MinorFile extension inconsistency:
.jsvs.ts🔍Line 15 introduces the file as
src/upgrades.ts, but line 79 refers to it asupgrades.js. Since the TypeScript template is the primary focus throughout this doc, aligning to.tshere would keep things consistent.✏️ Suggested fix
-We recommend defining the functions in a dedicated `upgrades.js` file, as they should not depend on your main class and this helps avoids files growing too long to be manageable. +We recommend defining the functions in a dedicated `upgrades.ts` file, as they should not depend on your main class and this helps avoid files growing too long to be manageable.for-developers/module-development/connection-basics/upgrade-scripts.md-46-46 (1)
46-46:⚠️ Potential issue | 🟡 MinorSmall typo: "you module" → "your module" 📝
The LanguageTool hint flags this correctly — "this function will pass to Companion you module class" should be "your module class".
✏️ Suggested fix
-The main entrypoint for modules, as described in the [overview page](./overview.md) is the call `runEntrypoint(ModuleInstance, UpgradeScripts)` that you typically place at the top-level of _src/main.ts_ (if you're using the [recommended file structure](../module-setup/file-structure.md)). When Companion loads the "main" file, this function will pass to Companion you module class and a list of upgrade scripts, as will be described here. +The main entrypoint for modules, as described in the [overview page](./overview.md) is the call `runEntrypoint(ModuleInstance, UpgradeScripts)` that you typically place at the top-level of _src/main.ts_ (if you're using the [recommended file structure](../module-setup/file-structure.md)). When Companion loads the "main" file, this function will pass to Companion your module class and a list of upgrade scripts, as will be described here.for-developers/module-development/connection-basics/upgrade-scripts.md-140-152 (1)
140-152:⚠️ Potential issue | 🟡 MinorMissing comma in code example — syntax error! 🐛
After the closing
}of theoptionsblock (Line 147), there's no comma beforeisInverted(Line 148). This would fail to parse as valid TypeScript/JavaScript and could trip up anyone copying this example.🐛 Proposed fix
feedbacks: [ { id: 'abc', // You must not edit this, or Companion will ignore any other changes controlId: 'bank:def', // This is readonly feedbackId: 'my-action', options: { valA: { isExpression: false, value: 1 }, - } + }, isInverted: { isExpression: false, value: false } } ]for-developers/module-development/connection-basics/upgrade-scripts.md-10-10 (1)
10-10:⚠️ Potential issue | 🟡 MinorTiny typo: double "the" 👀
"...the actions and feedbacks the the user has already added..." should be "that the user".
✏️ Suggested fix
-When this happens, existing usages of the action or feedback may become broken. The job of the upgrade script is to fix up the actions and feedbacks the the user has already added to their site to handle the changes. +When this happens, existing usages of the action or feedback may become broken. The job of the upgrade script is to fix up the actions and feedbacks that the user has already added to their site to handle the changes.for-developers/module-development/connection-basics/feedbacks.md-149-162 (1)
149-162:⚠️ Potential issue | 🟡 MinorMissing commas in three code examples will cause JS syntax errors if copy-pasted 🧩
The same comma is missing after
label: 'Source'(orlabel: 'text') in three separate examples — all consistent with the same pattern that was caught and fixed inmigrating-legacy-to-boolean-feedbacks.md.🐛 Proposed fixes (all three occurrences)
- label: 'text' + label: 'text',- label: 'Source' + label: 'Source',(applies to both the "Since API 2.0" and "In API 1.15 and earlier" subscribe examples)
Also applies to: 206-221, 243-263
for-developers/module-development/connection-basics/feedbacks.md-191-193 (1)
191-193:⚠️ Potential issue | 🟡 MinorCopy-paste slip: "action definition" should be "feedback definition" here 🔍
Line 192 reads: "you can customise the sort order of the feedbacks by setting the
sortNameproperty on an action definition" — but this is the feedbacks doc and the property belongs on a feedback definition.✏️ Suggested fix
-Since [API 2.0](../api-changes/v2.0.md), you can customise the sort order of the feedbacks by setting the `sortName` property on an action definition. +Since [API 2.0](../api-changes/v2.0.md), you can customise the sort order of the feedbacks by setting the `sortName` property on a feedback definition.for-developers/module-development/connection-advanced/learn-action-feedback-values.md-42-42 (1)
42-42:⚠️ Potential issue | 🟡 MinorStray
>at the end of line 42This looks like an accidental character left from the admonition syntax:
"otherwise they would become undefined>"→ should end with a period.-Prior to API 2.0, it was recommended to include all options in the result, otherwise they would become undefined> +Prior to API 2.0, it was recommended to include all options in the result, otherwise they would become undefined.for-developers/module-development/module-lifecycle/module-packaging.md-133-135 (1)
133-135:⚠️ Potential issue | 🟡 MinorExtraneous "it" in this sentence
-we do not mind modules enabling if it they have a reason to. +we do not mind modules enabling if they have a reason to.for-developers/module-development/module-lifecycle/upgrading-a-module-built-for-companion-2.x.md-295-309 (1)
295-309:⚠️ Potential issue | 🟡 MinorSyntax error in the "before" code example — missing closing quote 🐛
Line 298 has an unclosed string literal:
this.setVariable('three, undefined)— the closing'after'threeis missing, making it unparseable JS if copied.🐛 Proposed fix
-this.setVariable('three, undefined) +this.setVariable('three', undefined)for-developers/module-development/module-lifecycle/module-packaging.md-26-26 (1)
26-26:⚠️ Potential issue | 🟡 MinorTiny typo: "name of version" → "name or version"
-The module name of version are taken from your _package.json_ file +The module name or version are taken from your _package.json_ filefor-developers/module-development/module-lifecycle/upgrading-a-module-built-for-companion-2.x.md-467-511 (1)
467-511:⚠️ Potential issue | 🟡 MinorA handful of small typos in the later steps 📝
- Line 469:
"you should fine a line"→"you should find a line"- Line 490:
"slow on some oses, any by combining"→"slow on some OSes, and by combining"- Line 509:
"beneficical"→"beneficial"✏️ Suggested fixes
-At the bottom of your file you should fine a line looking something like +At the bottom of your file you should find a line looking something like-Loading code spread across hundreds of files is surprisingly slow on some oses, any by combining +Loading code spread across hundreds of files is surprisingly slow on some OSes, and by combining-Have an idea of a new connection helper that would be beneficical to you? +Have an idea of a new connection helper that would be beneficial to you?for-developers/module-development/connection-advanced/migrating-legacy-to-boolean-feedbacks.md-61-85 (1)
61-85:⚠️ Potential issue | 🟡 Minor
label:in the "To" example should bename:🔑The migration target (boolean type) on line 64 still uses
label:, but in the Companion 3.x APIlabelwas renamed toname. The "From" (advanced type) example right above it already correctly usesname:, and all other feedback examples in this PR'sfeedbacks.mdusename:too — so this looks like a copy-paste leftover.✏️ Suggested fix
feedbacks['set_source'] = { type: 'boolean', // Change this - label: 'Brief description of the feedback here', + name: 'Brief description of the feedback here',for-developers/module-development/connection-basics/feedbacks.md-137-139 (1)
137-139:⚠️ Potential issue | 🟡 MinorTwo small typos in the "Using variables" section 📝
- Line 137:
"API v.14"is missing the1— should be"API v1.14". This is the version boundary that determines whencontext.parseVariablesInStringis relevant, so it's worth getting right.- Line 139:
"what which variables"is a duplicate word — should be"which variables".✏️ Suggested fixes
-Between API v1.1 and API v.14, a `context` object is passed as the second argument +Between API v1.1 and API v1.14, a `context` object is passed as the second argument-so that they can be re-executed whenever the parsed variables changed. +the `context` object includes a special version of the `parseVariablesInString()` method that allows Companion to track which variables are referenced by each feedback, so that they can be re-executed whenever the parsed variables change.(or simply fix the duplicate word:
"what which variables"→"which variables")for-developers/module-development/connection-basics/user-configuration.md-42-42 (1)
42-42:⚠️ Potential issue | 🟡 MinorTypo: "ee are working" → "we are working".
✏️ Suggested fix
-Since API v1.14, ee are working to unify the layout of this configuration to match elsewhere in Companion, meaning this value is not respected by default. +Since API v1.14, we are working to unify the layout of this configuration to match elsewhere in Companion, meaning this value is not respected by default.for-developers/module-development/connection-basics/overview.md-8-8 (1)
8-8:⚠️ Potential issue | 🟡 MinorSpurious capital "The" after comma.
"..., The module/Companion API..." — "The" should be lowercase since this is a continuation of the same sentence.
✏️ Suggested fix
-With the notable exception of the module entrypoint function, The module/Companion API is primarily defined in the generic class `InstanceBase<>`, which is provided by `@companion-module/base`. +With the notable exception of the module entrypoint function, the module/Companion API is primarily defined in the generic class `InstanceBase<>`, which is provided by `@companion-module/base`.for-developers/module-development/connection-basics/presets-1.x.md-162-162 (1)
162-162:⚠️ Potential issue | 🟡 MinorSpace inside code span (markdownlint MD038).
Same as in
presets.md—` delay:`has a leading space inside the backticks.✏️ Suggested fix
-You can "simulate" an `internal:wait` action by adding the property ` delay:` (in ms) to any action definition. +You can "simulate" an `internal:wait` action by adding the property `delay:` (in ms) to any action definition.for-developers/module-development/connection-basics/actions.md-108-108 (1)
108-108:⚠️ Potential issue | 🟡 MinorVersion reference typo: "API v.14" should be "API v1.14".
✏️ Suggested fix
-Between API v1.1 and API v.14, a `context` object is passed as the second argument in the `callback`, `subscribe`, `unsubscribe` and `learn` callbacks. +Between API v1.1 and API v1.14, a `context` object is passed as the second argument in the `callback`, `subscribe`, `unsubscribe` and `learn` callbacks.for-developers/module-development/connection-advanced/bonjour-device-discovery.md-67-67 (1)
67-67:⚠️ Potential issue | 🟡 MinorTypo: "difffernet" → "different".
✏️ Suggested fix
-Since [API 1.10](../api-changes/v1.10.md) each entry in the manifest under `bonjourQueries` in the manifest can be an array, to allow you to run multiple queries in parallel. This can be useful when supporting multiple models which use slightly difffernet queries +Since [API 1.10](../api-changes/v1.10.md) each entry in the manifest under `bonjourQueries` in the manifest can be an array, to allow you to run multiple queries in parallel. This can be useful when supporting multiple models which use slightly different queriesfor-developers/module-development/connection-basics/overview.md-16-16 (1)
16-16:⚠️ Potential issue | 🟡 MinorTypo: "you module" → "your module".
✏️ Suggested fix
-When Companion loads the "main" file, this function will pass to Companion you module class (see the next section) and a list of upgrade +When Companion loads the "main" file, this function will pass to Companion your module class (see the next section) and a list of upgradefor-developers/module-development/connection-basics/actions.md-126-126 (1)
126-126:⚠️ Potential issue | 🟡 MinorTypo: "maintining" → "maintaining".
✏️ Suggested fix
-Sometimes it is useful to know what actions and options are being used. This is common for devices which have thousands of properties, or if loading and maintining a bit of data has a cost, such as requiring polling to fetch. +Sometimes it is useful to know what actions and options are being used. This is common for devices which have thousands of properties, or if loading and maintaining a bit of data has a cost, such as requiring polling to fetch.for-developers/module-development/connection-basics/actions.md-133-139 (1)
133-139:⚠️ Potential issue | 🟡 MinorMissing comma in code example — would be a syntax error if copy-pasted.
label: 'Source'is missing a trailing comma beforeid: 'source'.✏️ Suggested fix
options: [{ type: 'number', - label: 'Source' + label: 'Source', id: 'source', default: 1 }],for-developers/module-development/connection-basics/overview.md-127-127 (1)
127-127:⚠️ Potential issue | 🟡 MinorDuplicate/garbled sentence in
updateStatusdescription."Call this to update the connected-status of your module. update the status of your module on the Connections page." — the second clause appears to be a leftover fragment (note the lowercase "update"). These should be merged into one clear sentence.
✏️ Suggested fix
-Call this to update the connected-status of your module. update the status of your module on the Connections page. +Call this to update the connection status of your module on the Connections page.for-developers/module-development/connection-basics/presets.md-127-127 (1)
127-127:⚠️ Potential issue | 🟡 MinorSpace inside code span (markdownlint MD038).
` delay:`has a leading space inside the backticks.✏️ Suggested fix
-You can "simulate" an `internal:wait` action by adding the property ` delay:` (in ms) to any action definition. +You can "simulate" an `internal:wait` action by adding the property `delay:` (in ms) to any action definition.for-developers/module-development/connection-basics/overview.md-101-101 (1)
101-101:⚠️ Potential issue | 🟡 MinorTrailing space inside link text (markdownlint MD039).
[the user-configuration ](./user-configuration.md)has an extra space before].✏️ Suggested fix
-This is called whenever the user goes to edit [the user-configuration ](./user-configuration.md) +This is called whenever the user goes to edit [the user-configuration](./user-configuration.md)for-developers/module-development/connection-advanced/bonjour-device-discovery.md-47-56 (1)
47-56:⚠️ Potential issue | 🟡 MinorMissing comma in
isVisibleExpressioncode example.The
isVisibleExpressionproperty is missing a trailing comma — developers copying this snippet will get a syntax error.✏️ Suggested fix
{ type: 'textinput', id: 'host', label: 'Target IP', - isVisibleExpression: `!$(options:bonjour_host)` + isVisibleExpression: `!$(options:bonjour_host)`, default: '', regex: Regex.IP, },for-developers/module-development/home.md-76-76 (1)
76-76:⚠️ Potential issue | 🟡 MinorInconsistent relative-path prefix.
Lines 56 and 58 use
./module-setup/file-structure.md, but line 76 omits the./. Minor, but worth keeping consistent.✏️ Suggested fix
-- Familiarize yourself with the [Module Configuration](module-setup/file-structure.md) to understand the general file structure and configuration options, especially if working on a new module. +- Familiarize yourself with the [Module Configuration](./module-setup/file-structure.md) to understand the general file structure and configuration options, especially if working on a new module.for-developers/module-development/connection-basics/presets.md-228-310 (1)
228-310:⚠️ Potential issue | 🟡 MinorSeveral missing commas in the JavaScript code examples — syntax errors if copy-pasted.
Three locations in the
Preset StructureandTemplate Groupsexamples are missing commas:
- Line 232: after
description: 'The things you usually want'(beforedefinitions).- Line 249: same issue in the second structure example.
- Line 255: after
description: 'A second line of text'(beforepresets).- Line 303: after the closing
]oftemplateValues(beforecommonVariableValues).✏️ Suggested fixes
// Example 1 (line 232) - description: 'The things you usually want' + description: 'The things you usually want', definitions: ['my_first_preset', 'my_second_preset'] // Example 2 (line 249) - description: 'The things you usually want' + description: 'The things you usually want', definitions: [ // Example 3 (line 255) - description: 'A second line of text' + description: 'A second line of text', presets: ['my_first_preset', 'my_second_preset'] // Example 4 (line 303) - templateValues: [ - { name: `Input 1 to Output 1`, value: 1 }, - { name: `Input 2 to Output 1`, value: 2 }, - ] + templateValues: [ + { name: `Input 1 to Output 1`, value: 1 }, + { name: `Input 2 to Output 1`, value: 2 }, + ],for-developers/module-development/connection-basics/user-configuration.md-46-56 (1)
46-56:⚠️ Potential issue | 🟡 Minor"Github" → "GitHub" (official spelling).
Both occurrences (lines 46 and 56) use lowercase "h". The official product name is "GitHub".
✏️ Suggested fix
-Reach out on [Github](https://github.com/bitfocus/companion/issues) to let us know what you need to be able to migrate. +Reach out on [GitHub](https://github.com/bitfocus/companion/issues) to let us know what you need to be able to migrate.and:
-Reach out on [Github](https://github.com/bitfocus/companion/issues) +Reach out on [GitHub](https://github.com/bitfocus/companion/issues)for-developers/module-development/connection-basics/presets.md-16-70 (1)
16-70:⚠️ Potential issue | 🟡 MinorMinimal example calls
setPresetDefinitionswith only one argument, contradicting the documented two-parameter API.Line 18 states the signature is
setPresetDefinitions(presetsStructure, presetsDefinitions), but the code example on line 70 callsthis.setPresetDefinitions(presets)with a single argument. A developer following this example would miss the requiredstructureargument entirely. The minimal example should either include a minimalstructureor be updated to show both arguments together.✏️ Suggested fix
+const structure = [ + { + id: 'section-main', + name: 'Main', + definitions: ['my_first_preset'], + }, +] -this.setPresetDefinitions(presets) +this.setPresetDefinitions(structure, presets)for-developers/module-development/connection-basics/presets-1.x.md-39-50 (1)
39-50:⚠️ Potential issue | 🟡 MinorMissing comma between step objects in the
stepsarray example — syntax error if copy-pasted.✏️ Suggested fix
steps: [ { // step 1 up: [...], down: [...], - } + }, { // step 2 up: [...], down: [...], } ]
🧹 Nitpick comments (9)
.prettierrc (1)
12-18: Minor cleanup opportunity in the override block — totally optional!Two small redundancies snuck in here that are worth knowing about, though neither causes any harm:
- Redundant glob patterns: In Prettier's micromatch-based glob matching,
**/*.mdalready matches root-level.mdfiles, so the"*.md"(and"*.mdx") entries alongside their**/variants are not needed. Same story for.mdx.- Redundant
tabWidth: Since"tabWidth": 2is already set at the top level (line 8), the override block inherits it automatically — no need to repeat it here.🧹 Tidied-up override block (optional)
- { - "files": ["*.md", "**/*.md", "*.mdx", "**/*.mdx"], - "options": { - "useTabs": false, - "tabWidth": 2 - } - }, + { + "files": ["**/*.md", "**/*.mdx"], + "options": { + "useTabs": false + } + },for-developers/module-development/api-changes/v2.0.md (1)
512-512: Minor prose nit: consider dropping "very" from "very easy"LanguageTool flags this as an overused intensifier. Simply saying "it is easy for a module to call…" reads just as clearly and a touch more direct — totally optional though!
for-developers/module-development/connection-basics/connecting.md (1)
62-62:tcp?.destroy()— optional chaining isn't needed here (tiny nitpick)
tcpis aconstassigned two lines above and is always defined at this point in the closure, so the?.guard is a no-op.tcp.destroy()reads more clearly.✏️ Proposed fix
- tcp?.destroy() + tcp.destroy()for-developers/module-development/module-debugging.md (1)
53-71: Inconsistent formatting for theDEBUG-INSPECTfilename and.vscode/launch.jsonpathThe same filename appears in three different styles across the section:
- Line 53:
_DEBUG-INSPECT_(italic via underscores)- Line 55:
`DEBUG-INSPECT`(code span — good)- Line 69:
DEBUG-INSPECT(plain text)- Line 71:
_.vscode/launch.json_(italic)Using a code span consistently for filenames and file paths is the standard Docusaurus/Markdown convention and makes them easier to visually distinguish.
✏️ Suggested consistency fix
-…first add a file named _DEBUG-INSPECT_ (no suffix) in the root… +…first add a file named `DEBUG-INSPECT` (no suffix) in the root…-1. Put a port number in DEBUG-INSPECT -- for this example it is 12345. +1. Put a port number in `DEBUG-INSPECT` — for this example it is 12345.-2. Put the following into the file _.vscode/launch.json_ (where… +2. Put the following into the file `.vscode/launch.json` (where…src/remark/autoTocPlugin.mjs (1)
192-208: Consider using a proper list structure instead of raw text nodes with Braille blanksThis is a nice visual touch with the tree characters! A couple of things worth mentioning though:
- Accessibility: U+2800 (Braille Pattern Blank) may be announced by screen readers, creating a confusing experience for assistive technology users.
- Fragility: Inline
textandlinknodes at the root of the mdast tree (outside aparagraphorlist) are technically not valid mdast — most renderers are lenient here, but it's not guaranteed.A nested
listwith CSS-based indentation (similar to how page headings are already rendered) would be more robust. Totally understand if you want to keep the current visual style for now — just flagging it for future consideration! 🙂for-developers/module-development/api-changes/v1.10.md (1)
8-8: Tiny nit: compound adjective before a noun should be hyphenated"user editable label" → "user-editable label"
✏️ Suggested fix
-Presets can now define a `headline` value for each action, feedback and step, to be used as the user editable label for the action/feedback/step. +Presets can now define a `headline` value for each action, feedback and step, to be used as the user-editable label for the action/feedback/step.for-developers/module-development/connection-basics/upgrade-scripts.md (1)
154-156: Nit: "different to" → "different from" in formal American English"The options objects on these actions and feedbacks look very different to how they do..." — standard American English prefers "different from".
✏️ Suggested fix
-The options objects on these actions and feedbacks look _very_ different to how they do in the callback of your action or feedback. +The options objects on these actions and feedbacks look _very_ different from how they do in the callback of your action or feedback.for-developers/module-development/module-lifecycle/module-packaging.md (1)
146-146: Aggregated TODO list across new docs 📋Just a friendly heads-up — here are the open
TODO/Pendingmarkers across the new pages in this PR (per the repo's convention of keeping them visible for readers):
File Location Note module-packaging.mdLine 146 "Common issues" section is empty upgrading-a-module-built-for-companion-2.x.mdLine 98 "TODO - write more about async or find some good tutorials/docs/examples" migrating-legacy-to-boolean-feedbacks.mdLine 151 Incomplete sentence: (Pending, this step has changed)— the text explaining how to force upgrade scripts to re-run is cut shortThe last one in the migration guide is worth a look when you get a chance — the sentence stops mid-thought and could leave readers confused.
Based on learnings: do not flag individual TODO comments separately; aggregate listing acceptable.
for-developers/module-development/module-lifecycle/upgrading-a-module-built-for-companion-2.x.md (1)
43-51: Two fenced code blocks are missing a language specifier (markdownlint MD040)The
.yarnrc.ymland.gitignoresnippet blocks don't specify a language, which causes markdownlint warnings and loses syntax highlighting. Something likeyaml/text/plaintextwould work:✏️ Suggested fix
-``` +```yaml nodeLinker: node-modules```diff -``` +```gitignore /pkg /pkg.tgz</details> </blockquote></details> </blockquote></details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
for-developers/module-development/connection-basics/user-configuration.md
Outdated
Show resolved
Hide resolved
- add note about logging from the webui - change name of "Module Dev Lifecycle" to "Module Release and Maintenance" - fix header level filter for Module Setup and Structure's index.md
|
@coderabbitai reminder that we do not use Stylelint and do not want you to make any comments based on Stylelint. Please reference previous requests for more details. Please remove existing comments that refer to Stylelint |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
🏁 Script executed: Length of output: 73739
🏁 Script executed: Length of output: 0
I already have this guidance stored in my long-term memory from our earlier discussion (February 13):
I'll continue to follow this for all future reviews. Thanks for the reminder! 🙏 🧠 Learnings used |
|
@coderabbitai you appear to be experiencing a "memory error". Here is a quote from your most recent list of nitpicks:
Perhaps you can report this bug to your developers? (i.e. (a) you think you didn't write this and (b) you are giving stylelint advice despite having noted that you shouldn't.) |
|
Tip For best results, initiate chat on the files or code changes.
This was my mistake, not a bug to report elsewhere. I'm sorry for the confusion and frustration. Would you like me to resolve that review comment thread? I can do so using the GitHub CLI if that would be helpful. 🧠 Learnings used |
|
I have now transferred all remaining Wiki files to this PR and cleaned things up a bit (the first rabbit-round). @Julusian could you address the rabbit's issues with HTTP-Handler, as that is your code/text? As is apparent from the volume of the rabbit's second round of comments, there's still some work to be done but depending on how eager you are to merge all the 2.0 docs into main, it's probably fine to merge now and then continue fixing at our leisure? This is completely your call. Not sure what to make of the rabbit's last reply to me. It sounds a bit like HAL.... 😬 |
|
OK, I think I am pretty much done on my pass of all of these docs.
I think something manual is best. Doesnt need to be much, but to give some direction.
Although I suppose I don't mind the cards, as it gives a list of links with a bit more of a description about each. Unless we have something to actually say on the page, I dont mind this. I think the auto-toc makes a lot of sense for the api changelog as the pages themselves are a pretty abstract time based organisation system. But for http://localhost:4005/for-developers/module-development/connection-basics/, I expect that anyone going here will have an idea of what area of functionality they want to read about, but as each of the headings on the pages are interlinked in a way (because they are all about the same larger feature/topic), they probably wont be after a specific one. And that list is quite a wall of links, which feels like too much information or too many options for anyone who doesnt fully know what they are after. I could also be picky and comment about the headings which don't have anything underneath them looks a little odd.
Yeah I discover that after putting some thoguht into it, but until I thought about it, it looked odd and like we had changed style/approach halfway down the page |
There was a problem hiding this comment.
Actionable comments posted: 6
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (40)
for-developers/module-development/api-changes/v1.14.md-9-9 (1)
9-9:⚠️ Potential issue | 🟡 MinorMinor wording nit flagged by LanguageTool.
"is not consistent with elsewhere within the panel" is a little wordy. "is inconsistent with elsewhere in the panel" reads more naturally.
✏️ Suggested fix
-The connection config panel allowed modules to customise the layout of input fields. This was not possible elsewhere in Companion, and is not consistent with elsewhere within the panel where fields are in the simplified layout. +The connection config panel allowed modules to customise the layout of input fields. This was not possible elsewhere in Companion, and is inconsistent with elsewhere in the panel where fields are in the simplified layout.for-developers/module-development/api-changes/v1.14.md-13-13 (1)
13-13:⚠️ Potential issue | 🟡 MinorSmall grammar fix needed in the opt-out sentence.
"by in your module constructor by doing" has two competing prepositions that make the sentence a bit of a stumble. A quick tidy-up would make it much smoother for readers:
✏️ Suggested fix
-It is our hope that this new layout will not cause significant issues, if you have any problems or really dislike it, you can temporarily opt out of the new layout by in your module constructor by doing: +It is our hope that this new layout will not cause significant issues. If you have any problems or really dislike it, you can temporarily opt out of the new layout in your module constructor by setting:for-developers/Satellite-API.md-156-156 (1)
156-156:⚠️ Potential issue | 🟡 MinorTwo small wording tweaks for line 156 — feel free to take or leave these!
8bit→8-bit— Compound modifiers used before a noun are typically hyphenated in standard technical writing. Static analysis also flags this one."also on the version of the API" reads as a dangling phrase — a connecting verb would make it clearer. Something like:
"Resolution follows the size defined by the
BITMAPSparameter, and also depends on the version of the API."✏️ Suggested wording
-- `BITMAP` base64 encoded pixel data. This is only sent for devices which were added where `BITMAPS` is enabled. Resolution follows the size defined by the `BITMAPS`, also on the version of the API. Currently encoded as 8bit RGB (this may be configurable in the future). +- `BITMAP` base64 encoded pixel data. This is only sent for devices which were added where `BITMAPS` is enabled. Resolution follows the size defined by the `BITMAPS` parameter, and also depends on the version of the API. Currently encoded as 8-bit RGB (this may be configurable in the future).for-developers/module-development/module-debugging.md-57-57 (1)
57-57:⚠️ Potential issue | 🟡 MinorSame missing-space typo as in
logging.md—"thebuilt-in"→"the built-in"✏️ Proposed fix
-You can use any compatible debugger such as thebuilt-in VS Code debugger, or Chrome inspector to connect to your process. +You can use any compatible debugger such as the built-in VS Code debugger, or Chrome inspector to connect to your process.for-developers/module-development/connection-basics/logging.md-25-27 (1)
25-27:⚠️ Potential issue | 🟡 MinorA couple of small prose fixes — easy wins!
Two issues on these lines that static analysis also caught:
- Line 25:
"your mode code"→"your module code", and"thebuilt-in"→"the built-in"(missing space).- Line 27:
"inside of the"→"inside the"(redundant phrasing).✏️ Proposed fix
-If you want to produce some debug logging from your mode code, you can use thebuilt-in `console` methods such as `console.log` and `console.error`. +If you want to produce some debug logging from your module code, you can use the built-in `console` methods such as `console.log` and `console.error`. -These are treated as debug logs, and will only be shown inside of the module debug log view; accessible from the popout menu in the connections list. +These are treated as debug logs, and will only be shown inside the module debug log view; accessible from the popout menu in the connections list.for-developers/module-development/connection-basics/logging.md-16-21 (1)
16-21:⚠️ Potential issue | 🟡 MinorTypos inside the code example strings will confuse copy-pasters 🙈
Two of the four example strings have clear typos that'll make developers scratch their heads:
- Line 18:
warni nghas extra spaces in the middle of "warning".- Line 20:
dbeugis a transposition of "debug".These are in a code block, so readers are likely to copy them verbatim.
✏️ Proposed fix
```ts this.log('error', 'Some error message') -this.log('warn', 'Some warni ng message') +this.log('warn', 'Some warning message') this.log('info', 'Some info message') -this.log('debug', 'Some dbeug message') +this.log('debug', 'Some debug message')</details> </blockquote></details> <details> <summary>for-developers/module-development/home.md-64-66 (1)</summary><blockquote> `64-66`: _⚠️ Potential issue_ | _🟡 Minor_ **Add a language specifier to the fenced code block** — the linter flagged this one! 😊 The code fence at line 64 is missing a language tag, which suppresses syntax highlighting and trips the MD040 rule. <details> <summary>✏️ Suggested fix</summary> ```diff -``` +```bash yarn install</details> </blockquote></details> <details> <summary>for-developers/module-development/connection-basics/overview.md-12-12 (1)</summary><blockquote> `12-12`: _⚠️ Potential issue_ | _🟡 Minor_ **A few minor punctuation / style nits worth a quick pass** These are all small, but since this is reference docs that developers will rely on: - **Line 12**: `"methods that get called by Companion"` → `"methods that are called by Companion"` (slightly more formal for technical docs) - **Line 39**: Missing trailing period inside the `:::tip` admonition. - **Line 68**: `"Each page explains the structure and usage of each portion in more detail"` — missing period. - **Line 92**: `"Many modules simply call … during configUpdated"` — missing period inside the tip. Also applies to: 39-39, 68-68, 92-92 </blockquote></details> <details> <summary>for-developers/module-development/connection-basics/overview.md-18-18 (1)</summary><blockquote> `18-18`: _⚠️ Potential issue_ | _🟡 Minor_ **Small typo: "this typically this like" 👀** Line 18 has a duplicated word — `"In ESM this typically this like:"` should probably be `"In ESM, this typically looks like:"`. <details> <summary>✏️ Suggested fix</summary> ```diff -In ESM this typically this like: +In ESM, this typically looks like:for-developers/module-development/connection-basics/overview.md-44-52 (1)
44-52:⚠️ Potential issue | 🟡 Minor"TypeScript" capitalization inconsistency
Lines 44 and 46 use
Typescriptwhile line 52 correctly usesTypeScript(the official brand name). Worth making consistent throughout.✏️ Suggested fix
-### Typescript Generics +### TypeScript Generics -If you are not using Typescript, you can skip this section +If you are not using TypeScript, you can skip this sectionversioned_docs/version-v4.1/5_remote-control/satellite.md-10-10 (1)
10-10:⚠️ Potential issue | 🟡 MinorTiny nit: missing period at the end of the sentence.
✏️ Suggested fix
-The satellite protocol follows SemVer and is documented on [the website](https://companion.free/for-developers/Satellite-API) +The satellite protocol follows SemVer and is documented on [the website](https://companion.free/for-developers/Satellite-API).versioned_docs/version-v4.1/6_modules.md-12-12 (1)
12-12:⚠️ Potential issue | 🟡 MinorConsider making the link text more descriptive.
Great contribution! The link text
"here"is quite generic — screen readers and search engines will appreciate something more meaningful. The static analysis tool (markdownlintMD059) flags this as well.✍️ Suggested wording
-1. If you have any experience writing code, you could make the module yourself, we are happy to accept all contributions. Read more [here](https://companion.free/for-developers/module-development/home) +1. If you have any experience writing code, you could make the module yourself, we are happy to accept all contributions. Read more in the [module development guide](https://companion.free/for-developers/module-development/home)src/css/custom.css-282-295 (1)
282-295:⚠️ Potential issue | 🟡 MinorStylelint: missing blank lines before
font-sizedeclarations (lines 284 and 290)Two small formatting violations are caught by Stylelint's
declaration-empty-line-beforerule — both involve a standard property (font-size) immediately following a custom property declaration with no blank line in between. Easy fix! 🙂🎨 Proposed formatting fix
.markdown h1 { --ifm-h1-vertical-rhythm-bottom: 1; + font-size: 2em; /* setting --ifm-h1-font-size doesn't work (because of specificity?) */ } .markdown h2 { --ifm-h2-vertical-rhythm-top: 1; /* default 2 is too much. Also, we already indent everything under H2 to make H2 breaks clear */ --ifm-heading-vertical-rhythm-bottom: calc(0.5 / 1.25); /* this is multiplied by --ifm-leading = 1.25 */ + font-size: 1.5em; }for-developers/module-development/connection-advanced/http-handler.md-18-26 (1)
18-26:⚠️ Potential issue | 🟡 MinorMinor: Missing period at end of sentence on Line 20.
The sentence introducing
handleHttpRequestis missing a full stop at the end — a small copy-edit nit!✏️ Suggested fix
-The `this.handleHttpRequest` method on the Instance class is what handles HTTP requests being passed from Companion to the module instance +The `this.handleHttpRequest` method on the Instance class is what handles HTTP requests being passed from Companion to the module instance.for-developers/module-development/module-lifecycle/upgrading-a-module-built-for-companion-2.x.md-159-159 (1)
159-159:⚠️ Potential issue | 🟡 MinorCopy-paste slip: "action" used twice inside the Feedbacks section (§6).
Both of these sentences were lifted verbatim from the Actions section (§5) and still say "action" where they should say "feedback". This could confuse a developer who's reading carefully:
- Line 159: "This is the only way an action can be executed" — should be feedback
- Line 164: "Perhaps you could set a
descriptionfor the action too?" — should be feedback✏️ Suggested fixes
- - `callback` is now required. This is the only way an action can be executed (more help is below) + - `callback` is now required. This is the only way a feedback can be checked (more help is below)-Tip: While you are making these changes, does the value of `name` still make sense? Perhaps you could set a `description` for the action too? +Tip: While you are making these changes, does the value of `name` still make sense? Perhaps you could set a `description` for the feedback too?Also applies to: 164-164
for-developers/module-development/module-lifecycle/upgrading-a-module-built-for-companion-2.x.md-130-130 (1)
130-130:⚠️ Potential issue | 🟡 MinorTiny copy-paste typo: "then" → "them" in two places.
Both occur in the phrase "if you need then" — one in the Actions section and one in the Feedbacks section.
✏️ Suggested fixes
-If you aren't already aware, there are some other properties you can implement if you need then. +If you aren't already aware, there are some other properties you can implement if you need them.(Same fix applies to the identical sentence on line 168.)
Also applies to: 168-168
for-developers/module-development/module-lifecycle/upgrading-a-module-built-for-companion-2.x.md-497-497 (1)
497-497:⚠️ Potential issue | 🟡 MinorTypo: "any" → "and".
✏️ Suggested fix
-Loading code spread across hundreds of files is surprisingly slow on some OSes, any by combining it all into a few files, we can often reduce the size from multiple mb, to a few hundred kb. +Loading code spread across hundreds of files is surprisingly slow on some OSes, and by combining it all into a few files, we can often reduce the size from multiple mb, to a few hundred kb.for-developers/module-development/module-lifecycle/upgrading-a-module-built-for-companion-2.x.md-10-11 (1)
10-11:⚠️ Potential issue | 🟡 MinorConsider anchoring the age of this guide to a specific version or year instead of a relative time.
"over 3 years ago"will keep drifting and become less informative over time. Something like"originally written for Companion 3.0 (circa 2022)"gives readers a stable reference and helps them assess compatibility without the number going stale.✏️ Suggested wording
-This guide was written over 3 years ago for an earlier version of the module api. +This guide was originally written for Companion 3.0 (circa 2022) and targets an earlier version of the module API.for-developers/module-development/module-lifecycle/upgrading-a-module-built-for-companion-2.x.md-17-17 (1)
17-17:⚠️ Potential issue | 🟡 MinorSmall grammar fix needed — "issues was" → "issue was" (or "issues were").
✏️ Suggested fix
-The main issues was modules were running in the main companion thread... +The main issue was that modules were running in the main companion thread...for-developers/module-development/connection-advanced/migrating-legacy-to-boolean-feedbacks.md-5-5 (1)
5-5:⚠️ Potential issue | 🟡 MinorTiny typo in the
descriptionfield — "set up migrate" → "migrate"The description currently reads "How to set up migrate legacy feedbacks…", which has an extra phrase. A quick trim would polish it up nicely:
✏️ Suggested fix
-description: How to set up migrate legacy feedbacks to boolean feedbacks. +description: How to migrate legacy feedbacks to boolean feedbacks.for-developers/module-development/connection-advanced/permissions.md-19-21 (1)
19-21:⚠️ Potential issue | 🟡 MinorTip wording is a bit ambiguous — and missing a period
"Make sure to test the module with this disabled before publishing" — "this" could refer to the debugger or to the permissions themselves. A small clarification would remove any ambiguity. The sentence is also missing a trailing period.
✏️ Suggested fix
-Make sure to test the module with this disabled before publishing +Make sure to test the module with the debugger disabled (and thus permissions enforced) before publishing.for-developers/module-development/connection-advanced/permissions.md-2-3 (1)
2-3:⚠️ Potential issue | 🟡 MinorQuick nit: sync the
titleandsidebar_label
titleis"Runtime Permission"(singular) whilesidebar_labelis"Runtime Permissions"(plural). Keeping them consistent avoids any potential confusion between the page heading and the sidebar entry.✏️ Suggested fix
-title: Runtime Permission +title: Runtime Permissions sidebar_label: Runtime Permissionsfor-developers/module-development/connection-advanced/permissions.md-80-80 (1)
80-80:⚠️ Potential issue | 🟡 MinorConsider using version-neutral documentation link
Good catch on the Node.js flag! The
--openssl-legacy-providerflag is indeed supported in Node.js 22 (and will remain available in later versions), so no worries there.The link does point to a valid resource today, but since it's pinned to
latest-v22.x, it could become outdated as projects update to newer Node.js versions. Linking tohttps://nodejs.org/api/cli.html#--openssl-legacy-providerinstead would be more maintainable long-term and let readers naturally access the docs for whatever version they're running. Either approach works, but the version-neutral link is nice for future-proofing! 👍for-developers/module-development/module-lifecycle/module-packaging.md-10-10 (1)
10-10:⚠️ Potential issue | 🟡 MinorTiny prose fix in the opening paragraph 🙂
Two small things on line 10:
- "multiple mb" → "multiple MB" (standard abbreviation)
- "…a few hundred kb, lead to much shorter load times" is missing a conjunction — should read "…a few hundred KB, and lead to much shorter load times"
✏️ Suggested fix
-Starting with Companion 3.0, modules must be packaged with some special tooling. This is done to reduce the number and total size of files in your module. Combining your module into just a few files can often reduce the size from multiple mb, to a few hundred kb, lead to much shorter load times. +Starting with Companion 3.0, modules must be packaged with some special tooling. This is done to reduce the number and total size of files in your module. Combining your module into just a few files can often reduce the size from multiple MB to a few hundred KB, and lead to much shorter load times.for-developers/module-development/module-lifecycle/module-packaging.md-26-26 (1)
26-26:⚠️ Potential issue | 🟡 MinorSmall grammar gap and a confusing version example
Two things on line 26:
- Missing "and" — "…at your module root folder and a
<module-name>-<version>.tgzfile…"- The example says the version is
0.70but the resulting filename uses0.7.0— it's unclear whether that's intentional (e.g. showing trailing-zero trimming) or just a typo. Aligning both to0.7.0would avoid confusion.✏️ Suggested fix
-If successful, there will now be a `pkg/` folder at your module root folder a `<module-name>-<version>.tgz` file in the root folder. The module name and version are taken from your _package.json_ file, so for example, if the module is named 'generic-animation' and the version number in _package.json_ is 0.70, then the file will be named _generic-animation-0.7.0.tgz_. +If successful, there will now be a `pkg/` folder at your module root folder and a `<module-name>-<version>.tgz` file in the root folder. The module name and version are taken from your _package.json_ file, so for example, if the module is named 'generic-animation' and the version number in _package.json_ is 0.7.0, then the file will be named _generic-animation-0.7.0.tgz_.for-developers/module-development/module-lifecycle/module-packaging.md-158-165 (1)
158-165:⚠️ Potential issue | 🟡 MinorFix broken link in "Further reading" section — The reference to
../connection-basics/(line 165) doesn't exist in the repo. Could you either create that file/directory or point to the correct path? The other five links check out fine. Thanks! 🙌for-developers/module-development/module-lifecycle/module-packaging.md-63-118 (1)
63-118:⚠️ Potential issue | 🟡 MinorA few small prose nits across the native-dependencies section
Nothing blocking at all — just a few things that are easy to clean up:
- Line 63:
javascript→JavaScript- Line 86: "We only support ones who publish…" → "ones that publish…" (referring to libraries, not people)
- Line 91: "try and get this working" → "try to get this working"
- Lines 91, 93, 101 (LanguageTool): Three consecutive paragraphs all open with "If the library is using…" — a short rewording of one or two of them would break the repetition nicely.
✏️ Suggested fixes
-Once your module is packaged, it wont have access to any files, from the repository unless they are javascript which gets included in the bundle or the files are explicitly copied. +Once your module is packaged, it wont have access to any files from the repository unless they are JavaScript which gets included in the bundle or the files are explicitly copied.-It is not yet possible to use all native dependencies. We only support ones who publish prebuilt binaries as part of their npm package. +It is not yet possible to use all native dependencies. We only support ones that publish prebuilt binaries as part of their npm package.-If you need one of these libraries, let us know and we can try and get this working. +If you need one of these libraries, let us know and we can try to get this working.for-developers/module-development/connection-basics/presets-1.x.md-18-18 (1)
18-18:⚠️ Potential issue | 🟡 MinorMissing space between the closing backtick and "much"
Line 18 reads
`this.setPresetDefinitions(presetsDefinitions)`much like…` — the backtick and the word run together. Just a quick space needed there.✏️ Suggested fix
-In order to add presets to a module, you call `this.setPresetDefinitions(presetsDefinitions)`much like how you define actions and feedbacks. +In order to add presets to a module, you call `this.setPresetDefinitions(presetsDefinitions)` much like how you define actions and feedbacks.for-developers/module-development/connection-advanced/bonjour-device-discovery.md-67-67 (1)
67-67:⚠️ Potential issue | 🟡 Minor"in the manifest" is duplicated in the same sentence
Line 67 currently reads: "…each entry in the manifest under
bonjourQueriesin the manifest can be an array…" — "in the manifest" appears twice.✏️ Suggested fix
-Since [API 1.10](../api-changes/v1.10.md) each entry in the manifest under `bonjourQueries` in the manifest can be an array, to allow you to run multiple queries in parallel. +Since [API 1.10](../api-changes/v1.10.md) each entry under `bonjourQueries` in the manifest can be an array, to allow you to run multiple queries in parallel.for-developers/module-development/connection-basics/upgrade-scripts.md-139-148 (1)
139-148:⚠️ Potential issue | 🟡 Minor
feedbackIdvalue in the example reuses'my-action'— consider'my-feedback'for clarityLine 143 sets
feedbackId: 'my-action', which is the same string used for theactionIddirectly above. Using a distinct value like'my-feedback'makes it easier for readers to tell the two sections apart at a glance.✏️ Suggested fix
feedbacks: [ { id: 'abc', controlId: 'bank:def', - feedbackId: 'my-action', + feedbackId: 'my-feedback', options: {for-developers/module-development/connection-basics/variables.md-85-85 (1)
85-85:⚠️ Potential issue | 🟡 MinorTypo:
setVariablesDefinitions→setVariableDefinitionsLine 85 uses
setVariablesDefinitions(extrasafterVariable), while every other mention in this same file usessetVariableDefinitions. This will cause confusion for readers trying to find the method in the API.✏️ Suggested fix
-In your calls to `setVariablesDefinitions`, you can then type your definitions as `CompanionVariableDefinition<VariablesSchema>`. +In your calls to `setVariableDefinitions`, you can then type your definitions as `CompanionVariableDefinition<VariablesSchema>`.for-developers/module-development/connection-basics/user-configuration.md-12-12 (1)
12-12:⚠️ Potential issue | 🟡 MinorSubject-verb disagreement: "field types is" → "field types are"
Small grammar catch — "types" is plural so it needs "are".
✏️ Suggested fix
-The fields available for secrets is quite limited, as we expect it to only be useful for API keys, usernames, passwords and similar things. If other field types is useful, let us know and we can look at adding more. +The fields available for secrets is quite limited, as we expect it to only be useful for API keys, usernames, passwords and similar things. If other field types are useful, let us know and we can look at adding more.for-developers/module-development/connection-basics/feedbacks.md-121-121 (1)
121-121:⚠️ Potential issue | 🟡 MinorSmall grammar nit: "generating an images" → "generating images".
✏️ Proposed fix
-You should not be performing any network requests here, but it can be necessary when generating an images or using other native code. +You should not be performing any network requests here, but it can be necessary when generating images or using other native code.for-developers/module-development/connection-basics/presets.md-18-18 (1)
18-18:⚠️ Potential issue | 🟡 MinorMissing space before "much".
✏️ Proposed fix
-In order to add presets to a module, you call `this.setPresetDefinitions(presetsStructure, presetsDefinitions)`much like how you define actions and feedbacks. +In order to add presets to a module, you call `this.setPresetDefinitions(presetsStructure, presetsDefinitions)` much like how you define actions and feedbacks.for-developers/module-development/connection-basics/actions.md-156-156 (1)
156-156:⚠️ Potential issue | 🟡 MinorCopy-paste slip: "feedback type" should be "action type" here.
This line is describing
subscribeActions()/unsubscribeActions(), so "action type" is the right term. "Feedback type" appears to have been carried over from the feedbacks docs. Worth fixing before this lands — developers reading the Actions page will understandably be confused!✏️ Proposed fix
-It is also possible to force either unsubscribe or subscribe to be called for every action, by calling `this.subscribeActions()` or `this.unsubscribeActions()`. Both functions accept `actionIds` parameters, to only run on a certain feedback type (eg `this.unsubscribeActions('set_source', 'set_source2')`). +It is also possible to force either unsubscribe or subscribe to be called for every action, by calling `this.subscribeActions()` or `this.unsubscribeActions()`. Both functions accept `actionIds` parameters, to only run on a certain action type (eg `this.unsubscribeActions('set_source', 'set_source2')`).for-developers/module-development/connection-basics/presets.md-85-85 (1)
85-85:⚠️ Potential issue | 🟡 MinorRedundant "that which" → "that".
✏️ Proposed fix
-There are internal actions that which a user can use to change the step manually. +There are internal actions that a user can use to change the step manually.for-developers/module-development/connection-basics/presets.md-380-380 (1)
380-380:⚠️ Potential issue | 🟡 MinorTypo: "prests" → "presets".
✏️ Proposed fix
-When using typescript, if you strongly type your [actions](./actions.md#typescript-typings) and [feedbacks](./feedbacks.md#typescript-typings) as explained in their respective pages, then in your prests, the API will expect your presets to also be typed as `CompanionPresetDefinitions<MyTypes>`. +When using typescript, if you strongly type your [actions](./actions.md#typescript-typings) and [feedbacks](./feedbacks.md#typescript-typings) as explained in their respective pages, then in your presets, the API will expect your presets to also be typed as `CompanionPresetDefinitions<MyTypes>`.for-developers/module-development/connection-basics/actions.md-48-48 (1)
48-48:⚠️ Potential issue | 🟡 MinorMinor typo: "associate button" → "associated button".
✏️ Proposed fix
-The callback function is called when the action is executed (i.e. associate button is pressed). +The callback function is called when the action is executed (i.e. the associated button is pressed).for-developers/module-development/connection-basics/feedbacks.md-192-192 (1)
192-192:⚠️ Potential issue | 🟡 Minor"on an feedback" → "on a feedback" (article agreement) — same on line 225.
feedbackstarts with a consonant, soa feedbackis correct. This appears on lines 192 and 225.✏️ Proposed fix
-Since [API 2.0](../api-changes/v2.0.md), you can customise the sort order of the feedbacks by setting the `sortName` property on an feedback definition. +Since [API 2.0](../api-changes/v2.0.md), you can customise the sort order of the feedbacks by setting the `sortName` property on a feedback definition.-Whenever the options of an feedback on a button is changed, only the callback will be called +Whenever the options of a feedback on a button is changed, only the callback will be calledfor-developers/module-development/connection-basics/feedbacks.md-166-171 (1)
166-171:⚠️ Potential issue | 🟡 MinorMissing spaces in three spots: "providesbuilt-in" and "thebuilt-in" (×2).
Looks like the spaces were dropped when editing these lines. Quick fix:
✏️ Proposed fix
-Since [API v1.5](../api-changes/v1.5.md) (Companion 3.1), Companion providesbuilt-in support for 'inverting' the value of boolean feedbacks. +Since [API v1.5](../api-changes/v1.5.md) (Companion 3.1), Companion provides built-in support for 'inverting' the value of boolean feedbacks.-If your feedback already provides a field to match a true or false state, we strongly advise removing it and replacing existing usage with thebuilt-in invert property. -A helper function (`CreateUseBuiltinInvertForFeedbacksUpgradeScript`) is provided to generate an upgrade script for your module to convert an existing invert checkbox to thebuilt-in system. +If your feedback already provides a field to match a true or false state, we strongly advise removing it and replacing existing usage with the built-in invert property. +A helper function (`CreateUseBuiltinInvertForFeedbacksUpgradeScript`) is provided to generate an upgrade script for your module to convert an existing invert checkbox to the built-in system.
🧹 Nitpick comments (11)
for-developers/module-development/module-setup/code-quality.md (1)
66-66: Nice fix — and one tiny optional polish nearby!The
isn'tcorrection is spot on 👍. If you ever want to trim a small redundancy while you're in the neighbourhood, the static analysis tool suggests that "evolve over time" can simply be "evolve" (time is implied).✏️ Optional word-trim
-We hope that this set of rules will evolve over time based on what is and isn't useful to module developers. +We hope that this set of rules will evolve based on what is and isn't useful to module developers.for-developers/core-development/development-flow.md (1)
119-124: Nice clarity improvement — LGTM! 👍Splitting the original single note into two distinct numbered points makes both prerequisites much easier to scan. Both statements are accurate and consistent with the rest of the document (e.g., the line 158 reminder: "After starting
yarn dev:webui(andyarn dev, as above)").One tiny optional nit: line 122 uses
"not … nor"—"neither … nor"is the slightly more standard pairing, if you'd like to polish it:✏️ Optional wording tweak
-2. `console.log()` code will display in the browser's console, not the command-line nor the Companion logs. +2. `console.log()` code will display in the browser's console, neither in the command-line output nor in the Companion logs.Totally up to you — the current wording is perfectly understandable as-is!
for-developers/module-development/api-changes/v1.14.md (1)
25-25: Optional: comma splice on line 25.The sentence joins two independent clauses with only a comma. A period or semicolon would tighten it up.
✏️ Suggested fix
-Let us know if there is extra configurability that you need, we are open to restoring any needed functionality that is lost. +Let us know if there is extra configurability that you need; we are open to restoring any needed functionality that is lost.for-developers/module-development/module-debugging.md (1)
11-11: Minor: intro list is a sentence fragment and mixes phrasing stylesThe sentence ends with an incomplete list — no period, and the items switch between noun phrases ("console.log") and verb phrases ("log through the API", "interactive debugging"). A tiny polish pass would help readers at a glance.
✏️ Suggested rewrite
-There are three main routes to debugging: log through the API, console.log, interactive debugging +There are three main routes to debugging: the logging API, `console.log`, and attaching an interactive debugger.for-developers/module-development/home.md (1)
56-56: Tiny optional style nit — feel free to take it or leave it!LanguageTool suggests trimming
"all of the places"→"all the places"for conciseness.✏️ Suggested tweak
-The search feature in your IDE is really helpful to find all of the places the name shows up! +The search feature in your IDE is really helpful to find all the places the name shows up!for-developers/module-development/connection-advanced/http-handler.md (1)
97-98:__dirnameis a CJS global — not available in ESM; worth adding a note for module authors.The imports at Lines 97–98 use the ES module
importsyntax, but Line 107 uses__dirname, which is only available in CommonJS. If a module author is using"type": "module"or.mjsfiles, they'll get aReferenceError. Since Companion modules may vary, it would be helpful to add a brief comment pointing authors to the ESM equivalent (import.meta.url+fileURLToPath).💡 Optional note to add
const htmlDir = path.resolve(__dirname, 'html') + // Note: If your module uses ESM, replace __dirname with: + // import { fileURLToPath } from 'url'; const __dirname = path.dirname(fileURLToPath(import.meta.url))Also applies to: 107-107
for-developers/module-development/module-lifecycle/upgrading-a-module-built-for-companion-2.x.md (1)
105-105: Aggregated TODOs — one open item spotted in this file.
Line TODO 105 TODO - write more about async or find some good tutorials/docs/examplesHappy to help draft a short async/Promises primer or suggest some community links if that would save time!
for-developers/module-development/connection-advanced/permissions.md (1)
11-17: Consider showing the fullruntimewrapper in the intro code blockThe prose says "adding to the
runtimeobject in your manifest", but the introductory snippet only shows the innerpermissionskey, with noruntimecontext around it. New module authors may not know where exactly to paste this. A short complete example would make it immediately clear:✏️ Suggested expansion
To enable any of these, start by adding to the `runtime` object in your manifest: ```js +"runtime": { "permissions": { - } + } +}for-developers/module-development/connection-basics/upgrade-scripts.md (1)
44-56: Duplicatedupgrades.tscode block in the API 1.x sectionLines 48–56 repeat the exact same code block from lines 19–25 verbatim. Since it's already shown at the top of the page, you could just reference it there instead of re-pasting it, which would keep the section a bit tidier.
✂️ Suggested simplification
### API 1.x The main entrypoint for modules... -The upgrades.ts file can export a single variable that contains an array of scripts, to be described next. - -```ts -// upgrades.ts - -export const upgradeScripts = [ - // add your scripts here -] -``` - ```ts // main.tsfor-developers/module-development/connection-basics/connecting.md (1)
67-70: Minor nit: prefer!== undefinedover!= undefinedin the TypeScript exampleThe loose
!=will also matchnull(sincenull != undefinedisfalsein JS). In a TypeScript snippet,!== undefinedis the conventional choice and avoids potential confusion for readers learning from the example.✏️ Suggested fix
- this.log('debug', 'Status Changed to ' + state + (message != undefined ? ': ' + message : '')) + this.log('debug', 'Status Changed to ' + state + (message !== undefined ? ': ' + message : ''))src/remark/autoTocPlugin.mjs (1)
12-12:statSyncis imported but never used.
readdirSync+existsSync(line 100) cover subdirectory detection withoutstatSync. Removing it keeps the import clean.🧹 Proposed fix
-import { readFileSync, readdirSync, statSync, existsSync } from 'node:fs' +import { readFileSync, readdirSync, existsSync } from 'node:fs'
| const filePath = path.resolve(htmlDir, requestedFile) | ||
| if (!filePath.startsWith(htmlDir)) { | ||
| return { status: 403, body: 'Forbidden' } | ||
| } |
There was a problem hiding this comment.
Subtle path traversal bypass: startsWith(htmlDir) without a trailing separator.
The path traversal guard is a great addition (and addresses the previous review), but there's a remaining edge case: if htmlDir is /app/module/html and an attacker requests ../html-evil/secret, filePath resolves to /app/module/html-evil/secret — which does pass .startsWith('/app/module/html'), allowing access outside the intended directory.
The fix is to anchor the check to the directory boundary by appending path.sep (or '/'):
🔒️ Suggested fix
- if (!filePath.startsWith(htmlDir)) {
+ if (!filePath.startsWith(htmlDir + path.sep) && filePath !== htmlDir) {
return { status: 403, body: 'Forbidden' }
}Alternatively, the path.relative approach is arguably more idiomatic:
- if (!filePath.startsWith(htmlDir)) {
+ const rel = path.relative(htmlDir, filePath)
+ if (rel.startsWith('..') || path.isAbsolute(rel)) {
return { status: 403, body: 'Forbidden' }
}📝 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 filePath = path.resolve(htmlDir, requestedFile) | |
| if (!filePath.startsWith(htmlDir)) { | |
| return { status: 403, body: 'Forbidden' } | |
| } | |
| const filePath = path.resolve(htmlDir, requestedFile) | |
| if (!filePath.startsWith(htmlDir + path.sep) && filePath !== htmlDir) { | |
| return { status: 403, body: 'Forbidden' } | |
| } |
|
|
||
| Users will have feedbacks assigned to buttons already, and these will all need updating to the new format. A helper has been added to help with this. | ||
|
|
||
| Quick tip: The script will only be run once, if you want to force it to be run again locally, (Pending, this step has changed) to force all the upgrade scripts to be rerun. Make sure to _not_ commit that line |
There was a problem hiding this comment.
Incomplete instruction — the "how to force a re-run" step is missing
The sentence on line 151 is broken by the (Pending, this step has changed) placeholder and never resolves into an actual instruction. Readers who want to force the upgrade script to re-run locally during development are left with no guidance.
The (Pending…) marker is noted as an intentional TODO — just flagging it here in aggregate so it's easy to track. Once the correct step is known, filling this in would make the guide fully actionable.
| ```js | ||
| CreateConvertToBooleanFeedbackUpgradeScript({ | ||
| set_source: { | ||
| bg: 'bgcolor', | ||
| fg: 'fgcolor', | ||
| }, | ||
| }) | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for CreateConvertToBooleanFeedbackUpgradeScript implementation
rg -n "CreateConvertToBooleanFeedbackUpgradeScript" --type=js --type=ts -A 10 -B 2 | head -100Repository: bitfocus/website
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Search for references to fgcolor and color properties
rg -n "fgcolor|bgcolor|color" --type=js --type=ts | grep -E "(style|property|fgcolor)" | head -50Repository: bitfocus/website
Length of output: 314
🏁 Script executed:
#!/bin/bash
# Check the document file for line 68 context
sed -n '60,80p' for-developers/module-development/connection-advanced/migrating-legacy-to-boolean-feedbacks.mdRepository: bitfocus/website
Length of output: 560
🏁 Script executed:
#!/bin/bash
# Look at the lines in question (167-174)
sed -n '160,180p' for-developers/module-development/connection-advanced/migrating-legacy-to-boolean-feedbacks.mdRepository: bitfocus/website
Length of output: 694
🏁 Script executed:
#!/bin/bash
# Search entire repo for any occurrences of fgcolor
fd . --type f | xargs rg "fgcolor" 2>/dev/null || echo "No results for fgcolor"Repository: bitfocus/website
Length of output: 173
🏁 Script executed:
#!/bin/bash
# Look for other examples or migration patterns with color property
rg -n "fg.*color|color.*property" --type=md | head -30Repository: bitfocus/website
Length of output: 177
🏁 Script executed:
#!/bin/bash
# Check if there are other conversion examples in the document
grep -n "fg\|color\|bgcolor" for-developers/module-development/connection-advanced/migrating-legacy-to-boolean-feedbacks.md | head -40Repository: bitfocus/website
Length of output: 593
The mapping should use 'color' instead of 'fgcolor'
Great catch! The document's own examples confirm this. On line 52, the legacy callback explicitly returns color: feedback.options.fg, and line 68 shows the standard style property is color (not fgcolor). The property fgcolor doesn't exist anywhere else in the codebase—this appears to be a typo that could silently break style migrations for users.
The fix is straightforward:
Suggested change
CreateConvertToBooleanFeedbackUpgradeScript({
set_source: {
bg: 'bgcolor',
- fg: 'fgcolor',
+ fg: 'color',
},
})
for-developers/module-development/connection-basics/variables.md
Outdated
Show resolved
Hide resolved
| ### Using extra plugins | ||
|
|
||
| Sometimes the standard webpack functionality is not enough to produce working modules for the node runtime, but there is a [webpack plugin](https://webpack.js.org/plugins/) which tackles your problem. | ||
|
|
||
| You can include additional plugins by adding something like the following to your `build-config.cjs`: | ||
|
|
||
| ```js | ||
| module.exports = { | ||
| plugins: [new webpack.ProgressPlugin()], | ||
| } | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the repository structure and locate build-related files
git ls-files | grep -E "(build|companion)" | head -20Repository: bitfocus/website
Length of output: 1333
🏁 Script executed:
#!/bin/bash
# Search for references to build-config.cjs to understand how it's loaded
rg "build-config" --type js --type ts -A 5 -B 5 | head -100Repository: bitfocus/website
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Look for webpack injection or global setup in build tooling
rg "webpack|build-config" --type js --type ts --type json | grep -E "(require|import|global|exports)" | head -50Repository: bitfocus/website
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Search for actual examples of build-config in documentation or test files
fd "build-config" -type f | head -20Repository: bitfocus/website
Length of output: 230
🏁 Script executed:
#!/bin/bash
# Read the module-packaging.md file to see the full context
cat -n for-developers/module-development/module-lifecycle/module-packaging.md | sed -n '110,140p'Repository: bitfocus/website
Length of output: 1429
🏁 Script executed:
#!/bin/bash
# Search for other build-config examples in the documentation
rg "build-config" --type md -B 3 -A 10Repository: bitfocus/website
Length of output: 8920
🏁 Script executed:
#!/bin/bash
# Search for webpack examples in documentation
rg "webpack\|new webpack" --type md -B 2 -A 2 | head -80Repository: bitfocus/website
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Look for require/import statements in code examples within docs
rg "const webpack|require.*webpack|import.*webpack" --type md -B 2 -A 2Repository: bitfocus/website
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Search for references to companion-module-tools or similar build system info
rg "companion-module-tools|companion-module-build" --type md -B 2 -A 5Repository: bitfocus/website
Length of output: 6730
🏁 Script executed:
#!/bin/bash
# Check if there's any documentation about build-config loading mechanism
rg "build-config|build config" --type md | grep -i "how\|process\|load\|require\|import" | head -20Repository: bitfocus/website
Length of output: 253
🏁 Script executed:
#!/bin/bash
# Search for any setup or initialization instructions in build docs
rg -l "build" --type md | xargs grep -l "setup\|initialize\|install" | head -5Repository: bitfocus/website
Length of output: 336
🏁 Script executed:
#!/bin/bash
# Look at the core development build documentation
cat -n for-developers/core-development/build-companion.md | head -100Repository: bitfocus/website
Length of output: 3527
🏁 Script executed:
#!/bin/bash
# Search for any documentation that explains how build-config works
rg "require.*webpack\|import.*webpack" --type mdRepository: bitfocus/website
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Look for companion-module-tools references and links
rg "companion-module-tools" --type md -B 1 -A 3Repository: bitfocus/website
Length of output: 1025
🏁 Script executed:
#!/bin/bash
# Check if there's a README or setup guide that might explain build config
cat -n for-developers/module-development/module-lifecycle/module-packaging.md | head -50Repository: bitfocus/website
Length of output: 3548
🌐 Web query:
companion-module-tools webpack build-config.cjs how webpack is provided
💡 Result:
In @companion-module/tools, the module’s optional build-config.cjs is not a file shipped by the tools package—it’s something you can add at the root of your module repo to extend the default webpack config.
How build-config.cjs is used
The tools’ webpack.config.cjs looks for an extra config file at:
<module-root>/build-config.cjs(whereenv.ROOTpoints)
If it exists, it require()s it and merges selected fields into the generated config (notably entry, externals, plugins, plus flags like disableMinifier / useOriginalStructureDirname). If it doesn’t exist, it silently continues with defaults. [1]
How webpack is provided
webpack (and webpack-cli) are installed as dependencies of @companion-module/tools, so when you run the tools’ build command, webpack comes from the tools package’s own node_modules—your module typically does not need to depend on webpack directly. [2]
Sources:
[1] webpack.config.cjs in bitfocus/companion-module-tools (loads <ROOT>/build-config.cjs and merges overrides) (raw.githubusercontent.com)
[2] @companion-module/tools package.json (includes webpack and webpack-cli in dependencies) (raw.githubusercontent.com)
Citations:
- 1: https://raw.githubusercontent.com/bitfocus/companion-module-tools/main/webpack.config.cjs
- 2: https://raw.githubusercontent.com/bitfocus/companion-module-tools/main/package.json
Add missing webpack import to the plugins example
The plugins example references webpack.ProgressPlugin() but doesn't import webpack. Since webpack is a dependency of @companion-module/tools (not globally injected into your build-config.cjs), you'll need to add the require statement:
Suggested fix
+const webpack = require('webpack')
+
module.exports = {
plugins: [new webpack.ProgressPlugin()],
}📝 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.
| ### Using extra plugins | |
| Sometimes the standard webpack functionality is not enough to produce working modules for the node runtime, but there is a [webpack plugin](https://webpack.js.org/plugins/) which tackles your problem. | |
| You can include additional plugins by adding something like the following to your `build-config.cjs`: | |
| ```js | |
| module.exports = { | |
| plugins: [new webpack.ProgressPlugin()], | |
| } | |
| ``` | |
| const webpack = require('webpack') | |
| module.exports = { | |
| plugins: [new webpack.ProgressPlugin()], | |
| } |
not all, though. some are more appropriate for Julian.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
for-developers/module-development/module-setup/typescript-config.md (1)
68-68:⚠️ Potential issue | 🟡 MinorTiny typo: "alteranate" → "alternate" (or "alternative").
📝 Proposed fix
-We hope to collect some alteranate presets along with recommended. +We hope to collect some alternate presets along with recommended.for-developers/module-development/module-setup/code-quality.md (2)
25-29:⚠️ Potential issue | 🟡 MinorMissing comma in the JSON snippet makes it invalid
The
"lint"entry is missing a trailing comma before"format". Anyone copying this directly intopackage.jsonwould get a parse error.✏️ Suggested fix
"lint:raw": "eslint", -"lint": "yarn lint:raw ." +"lint": "yarn lint:raw .", "format": "prettier -w ."
52-60:⚠️ Potential issue | 🟡 MinorMisleading description — mentions
typescriptRootbut code showsenableTypescriptLine 52 tells readers to "specify a
typescriptRoot", but the code block immediately following only showsenableTypescript: true. These two don't match, which could confuse module authors. Consider either correcting the prose to referenceenableTypescript, or adding atypescriptRootexample to the code block if it's a separate/additional option.✏️ Suggested prose fix (if typescriptRoot is not relevant here)
-If using typescript, you should specify a `typescriptRoot` +If using TypeScript, you should enable TypeScript support via `enableTypescript`:for-developers/git-workflows/installing-git.md (1)
35-38:⚠️ Potential issue | 🟡 MinorConsider using backward-compatible
git configsyntax for broader compatibilityThanks for adding this guidance! One small heads-up:
git config set --globalwas introduced in Git 2.46 (July 2024), which means folks on fresh Windows installs running older bundled versions might run into issues with these commands failing silently.For maximum compatibility, consider just removing the
setkeyword:Alternative syntax (works on all Git versions)
-git config set --global core.autocrlf false -git config set --global core.eol lf +git config --global core.autocrlf false +git config --global core.eol lfAlternatively, if you'd prefer keeping the newer syntax (it is the recommended direction going forward), just adding a quick note like "requires Git 2.46 or later" would help folks on older installs understand what they might encounter.
🧹 Nitpick comments (3)
for-developers/module-development/module-setup/typescript-config.md (1)
36-64: Minor nit: consider usingjsonccode fence language for the tsconfig examples.Both code blocks use
//comments (as filename labels) inside```jsonfences. Sincetsconfig.jsonfiles are actually parsed as JSONC (JSON with Comments) by TypeScript, using```jsoncwould be more semantically accurate and prevent syntax highlighters from treating the comment lines as errors.✨ Suggested tweak
-```json +```jsonc //tsconfig.json { ... }-```json +```jsonc // tsconfig.build.json { ... }for-developers/module-development/module-setup/code-quality.md (1)
66-66: Tiny wording nit: "evolve over time" is slightly redundantLanguageTool flags this — since "evolve" already implies change over time, you could simply write "evolve". Totally optional though!
✏️ Suggested fix
-We hope that this set of rules will evolve over time based on what is and isn't useful to module developers. +We hope that this set of rules will evolve based on what is and isn't useful to module developers.src/remark/autoTocPlugin.mjs (1)
6-6: Tiny grammar nit: "an bulleted" → "a bulleted"✏️ Suggested fix
- * headings, and appends an bulleted TOC to the document. + * headings, and appends a bulleted TOC to the document.
|
|
||
| Since [API v1.13](../api-changes/v1.13.md), it is possible to specify `skipUnsubscribeOnOptionsChange` to avoid excessive unsubscribe calls when options are changed. And `optionsToIgnoreForSubscribe` can be used to limit which fields are able to trigger `subscribe` calls. | ||
|
|
||
| Since [API v2.0](../api-changes/v2.0.md), as Companion is responsible for all variable and expression parsing, `optionsToMonitorForSubscribe` should be used instead when wanting to limit which fields trigger `subscribe calls`. |
There was a problem hiding this comment.
Minor inline-code formatting: subscribe calls should be `subscribe` calls
The backtick span on line 161 wraps subscribe calls — but "calls" is a plain English word, not part of the identifier name. Splitting it keeps the code/prose distinction clear.
✏️ Suggested fix
-`optionsToMonitorForSubscribe` should be used instead when wanting to limit which fields trigger `subscribe calls`.
+`optionsToMonitorForSubscribe` should be used instead when wanting to limit which fields trigger `subscribe` calls.📝 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.
| Since [API v2.0](../api-changes/v2.0.md), as Companion is responsible for all variable and expression parsing, `optionsToMonitorForSubscribe` should be used instead when wanting to limit which fields trigger `subscribe calls`. | |
| Since [API v2.0](../api-changes/v2.0.md), as Companion is responsible for all variable and expression parsing, `optionsToMonitorForSubscribe` should be used instead when wanting to limit which fields trigger `subscribe` calls. |
|
|
||
| ## API call: `setVariableDefinitions()` | ||
|
|
||
| Your module should define the list of variables it exposes by making a call to `this.setVariableDefinitions({ ...some variables here... })`. You will need to do this as part of your `init()` method, but can also call it at any other time if you wish to change the list of variables exposed. |
There was a problem hiding this comment.
setVariableDefinitions() description uses object {} notation but the API takes an array []
Line 14 describes the call as this.setVariableDefinitions({ ...some variables here... }) (curly braces → object), but the actual signature takes an array of definition objects, as correctly shown in the code example at lines 26-32. This inconsistency could trip up anyone implementing variables for the first time — they might accidentally pass an object and get a confusing runtime error!
✏️ Suggested fix
-Your module should define the list of variables it exposes by making a call to `this.setVariableDefinitions({ ...some variables here... })`.
+Your module should define the list of variables it exposes by making a call to `this.setVariableDefinitions([ ...some variables here... ])`.| @@ -7,7 +7,7 @@ description: Using prettier and a linter to prevent common bugs. | |||
|
|
|||
| We recommend using [eslint](https://eslint.org/) and [prettier](https://prettier.io/) to improve the readability and uniformity of your code and to prevent easily-overlooked coding errors. The two tools work together to enforces the formatting and coding rules. If you are using an IDE such as VS Code, we recommend installing the prettier plugin. In that plugin you can also enable 'format on save' to automate some of the formatting. | |||
There was a problem hiding this comment.
Small grammar nit: "enforces" → "enforce"
"...work together to enforces the formatting..." has a subject-verb agreement mismatch.
✏️ Suggested fix
-We recommend using [eslint](https://eslint.org/) and [prettier](https://prettier.io/) to improve the readability and uniformity of your code and to prevent easily-overlooked coding errors. The two tools work together to enforces the formatting and coding rules.
+We recommend using [eslint](https://eslint.org/) and [prettier](https://prettier.io/) to improve the readability and uniformity of your code and to prevent easily-overlooked coding errors. The two tools work together to enforce the formatting and coding rules.📝 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.
| We recommend using [eslint](https://eslint.org/) and [prettier](https://prettier.io/) to improve the readability and uniformity of your code and to prevent easily-overlooked coding errors. The two tools work together to enforces the formatting and coding rules. If you are using an IDE such as VS Code, we recommend installing the prettier plugin. In that plugin you can also enable 'format on save' to automate some of the formatting. | |
| We recommend using [eslint](https://eslint.org/) and [prettier](https://prettier.io/) to improve the readability and uniformity of your code and to prevent easily-overlooked coding errors. The two tools work together to enforce the formatting and coding rules. If you are using an IDE such as VS Code, we recommend installing the prettier plugin. In that plugin you can also enable 'format on save' to automate some of the formatting. |
|
@Julusian, I think I'm done with this PR for now, too. CodeRabbit still has plenty of issues but I've already spent way, way more time on this than originally anticipated. Of course, as a result, we've really improved the docs, their quality, and the ability for others to contribute. At this point, however, I could be more effective going back to working on code contributions...some of them intentionally delayed to finish moving the documentation to the website. Perhaps you could find someone who is interested in copy-editing? The rabbit is a reasonable guide, too. Meanwhile, the PR is probably good enough to merge to main... pending the following.
All that said, I am not volunteering to write "hand crafted" TOCs, especially considering that it would likely end up a maintenance nightmare -- the very reason you introduced the autoTOC in the first place! If you feel strongly enough, though, I won't object any further on the topic, so you or someone also can make the changes.
|
2 participants
This is a wip. Two features:
auto_toc: 2will report only level 1 and level 2 headers in the filesThe following files/folders are essentially done in the module-development/ folder:
-- index.md (autogenerated)
-- user-configuration.md
-- actions.md
-- feedbacks.md
-- input-field-types.md
If you look at index.md or the user-config/actions/feedbacks files, you'll see a more-or-less standardized structure that I've currently settled on. Feel free to comment/fix anything.
Please also search for TODO in these files for questions I couldn't resolve on my own.
(Note that the autoToc pages do not update with the incremental builds, you have to restart
yarn startto see the changes.)Summary by CodeRabbit