feat: add JSDoc example #8533

linonetwo · 2024-08-20T13:24:00Z

Just an example yet.

github-actions · 2024-08-20T13:24:10Z

Confirmed: linonetwo has already signed the Contributor License Agreement (see contributing.md)

Jermolene

Jermolene · 2024-08-20T15:14:03Z

core/modules/parsers/wikiparser/rules/codeblock.js

@@ -1,4 +1,5 @@
-/*\
+// @ts-check


What does this do?

It enables VSCode and ts cli regard this file as .ts file. Maybe this enable different parse mode of VSCode.

The docs seem to imply that one can also use a separate "jsconfig.json" file. Would that be possible here?

I try again and find VScode works with or without this. What make it work is open both file as tab...

I need to do more experiment.

Jermolene · 2024-08-20T15:14:23Z

core/modules/parsers/wikiparser/rules/codeblock.js

@@ -1,4 +1,5 @@
-/*\
+// @ts-check
+/**


Do we have to change the existing /*\ pattern?

No, I find @module $:/core/modules/parsers/wikiparser/rules/codeblock.js is useless for us, so the comment here don't need to be JSDoc anymore.

But the comment for type needs to be valid jsdoc

Without JSDoc:

With JSDoc:

core/modules/parsers/wikiparser/rules/codeblock.js

pmario · 2024-09-09T13:59:46Z

@linonetwo -- After your latest changes, does VSCode show useful type info in the tooltips now?

linonetwo · 2024-09-10T02:26:43Z

@pmario Yes, I find // @ts-check is not necessary, but the tsconfig.json is necessary (And of cource also need the TypeScript plugin on your code editor).

I'm experiment with the tidddlywiki npm package, because this PR is intended for providing AST type for plugin developers.

…/core/modules/...')`. Only works inside this project.

linonetwo · 2024-09-10T08:16:24Z

Now $tw.wiki.xxx have type check and docs.

linonetwo · 2024-09-10T08:52:11Z

Solution to fix core/modules/widgets/widget.js(9,1): error TS9005: Declaration emit for this file requires using private name 'Widget'. An explicit type annotation may unblock declaration emit.

change

var Widget = function(parseTreeNode,options) {
	this.initialise(parseTreeNode,options);
};

to

function Widget(parseTreeNode,options) {
	this.initialise(parseTreeNode,options);
};

and add @class in the jsdoc.

pmario · 2024-09-10T08:36:59Z

core/modules/wiki.js

+
+/**
+ * @type {Widget}
+ */


Can the 2 comment blocks be merged to 1 block, or do we need 2 of them?

pmario · 2024-09-10T08:55:58Z

core/modules/wiki.js

+ * @param {string} title - The title of the tiddler.
+ * @param {string} [defaultText] - The default text to return if the tiddler is missing.
+ * @returns {string | null | undefined} - The text of the tiddler, undefined if the tiddler is missing, or null if the tiddler is being lazily loaded.
+ */


IMO the type text should be as short as possible, but still understandable. Your definition contains "undefined", which is not returned. So I would suggest:

/** * Returns the tiddler text-field. If field `_is_skinny` is present it triggers a "lazyLoad" event. * * @param {string} title - Tiddler title * @param {string} [defaultText] - Returned if tiddler is missing * @returns {string | null } - Returns the "text-field" or an empty string, "defaultText" if the tiddler is missing or null if the tiddler is being lazily loaded */

When add @returns, it means it always return, so can't remove undefined here. It is same in TypeScript.

pmario · 2024-09-10T09:51:22Z

core/modules/parsers/wikiparser/rules/codeblock.js

+ * @property {number} attributes.language.start - The start position of the language string in the source text.
+ * @property {number} attributes.language.end - The end position of the language string in the source text.
+ */
+


IMO having @property {string} type and then - The type of the widget, which is "codeblock". is the same thing. Both elements describe: "What it is". -- I think the second part should describe: "What it does", otherwise we can skip it.

@Property does describe -- What it is

Text describes -- What it does

eg:

/** * Represents the parser `codeblock` rule. * * @typedef {Object} CodeblockNode * @property {string} rule - Always "codeblock" * @property {string} type - Always "codeblock" * @property {number} start - Rule start marker in source text * @property {number} end - Rule end marker in source text * @property {Object} attributes - Contains "code" and "language" objects * @property {Object} attributes.code - The code attribute object * @property {string} attributes.code.type - Always "string" * @property {string} attributes.code.value - Actual code * @property {number} attributes.code.start - Start position of code in source text * @property {number} attributes.code.end - End position of code in source text * @property {Object} attributes.language - The language attribute object * @property {string} attributes.language.type - Always "string" * @property {string} attributes.language.value - Language specified after the triple backticks, if any * @property {number} attributes.language.start - Start position of the string in source text * @property {number} attributes.language.end - End position of the string in source text */

Should be as short as possible and still valid. Especially see type, rule, start, end which are properties of "CodeblockNode"

I did remove the "full stops" from all @ elements. They are not needed
The first intro sentence has a full stop.

Thanks, I'd copy that.

I have to admit, all jsdoc are generated by github copilot. I don't have time for this detail, I will leave it for future refinement. I'm still debugging the type when I'm using it in my plugin project.

I have to admit, all jsdoc are generated by github copilot. I don't have time for this detail, I will leave it for future refinement. I'm still debugging the type when I'm using it in my plugin project.

The new type information will significantly increase the TW code-size. So if there is redundant information it should be removed.

And even more important it has to be valid. So if the co-pilot only adds comments in a more verbose form than the parameters are. There should not be any comments at all -- They have no value.

So if there are no comments, we actually know, that we have to add them manually. IMO for the tooltips it would be OK to start with the type info.

will significantly increase the TW code-size

we must remove comments before publishing HTML wiki. I think these JSDoc will basically double the size.

it has to be valid

This won't be a big problem, because 1. I use the method body as input too. And 2. we can auto merge "comment" type of PR based on #7542 , so people can update comment quickly. I find this is quite frequent when maintaining https://github.com/tiddly-gittly/TW5-Typed

…et'. An explicit type annotation may unblock declaration emit.

linonetwo · 2024-09-10T11:34:18Z

Plugin developers can install typescript globally, and run tsc or rm -rf ./types/generated && tsc > log.log in ./node_modules/tiddlywiki of their plugin project, to generate type that can be used in the plugin.

But I'm still working on this, some imported type is not working currently.

pmario · 2024-09-10T19:30:14Z

core/modules/parsers/wikiparser/wikiparser.js

+ * @param {boolean} [options.parseAsInline=false] - If true, the text will be parsed as an inline run.
+ * @param {Object} options.wiki - Reference to the wiki to use.
+ * @param {string} [options._canonical_uri] - Optional URI of the content if the text is missing or empty.
+ * @param {boolean} [options.configTrimWhiteSpace=false] - If true, trims white space according to configuration.


IMO the full-stop at the end of the comment text can be removed.
IMO "The" and "the" can be removed -- most of the time. So it does not obscure the important content. eg:

* @param {string} type - Content type of the text to be parsed * @param {string} text - Text to be parsed * @param {Object} options - Parser options * @param {boolean} [options.parseAsInline=false] - If true, the text will be parsed as an inline run * @param {Object} options.wiki - Reference to the wiki store in use * @param {string} [options._canonical_uri] - Optional URI of the content if text is missing or empty * @param {boolean} [options.configTrimWhiteSpace=false] - If true, the parser trims white space

Thanks, this will be the updated prompt:

add jsdoc, only output jsdoc for this class and its method, no impl. full-stop at the end of the comment text can be removed. "The" and "the" can be removed -- most of the time. So it does not obscure the important content.

linonetwo · 2024-09-12T09:58:15Z

Now plugin developers can get type hint directly generated from TW source code:

1. tsconfig.json in plugin root

As a plugin developer, in your project:

  "compilerOptions": {
    "paths": {
      // Allow `import('$:/core/modules/...')` instead of `import('tiddlywiki/core/modules/...')`. Only works inside this project.
      "$:/core/*": ["node_modules/tiddlywiki/types/core/*"],
      "tiddlywiki/*": ["node_modules/tiddlywiki/types/*"],
    },

Don't need allowJs checkJs, because we are going to...

2. run `tsc` in `node_modules/tiddlywiki/`

Hope @Jermolene can do this before publish, otherwise we have to do it by ourself.

This will generate types/core/modules/ folder full of .d.ts file. So we can do

import type { ParseTreeNode } from 'tiddlywiki/types/core/modules/parsers/base';

I find it is impossible to directly import JSDoc from 'tiddlywiki/core/modules/parsers/base'; (without /types/), even open maxNodeModuleJsDepth in tsconfig.

3. enjoy the type

4. maintaining the type

I only add typing to some typical AST node in this PR. Also I enable type of $tw.wiki.xxx to show this can be helpful not only to plugin developers, but also to core developers.

Now I need to know your opinion before I spend more 10h+ time to this.

Jermolene

Thank you @linonetwo. I am concerned that there is a lot of overhead required, and that overhead is not limited to users working in TypeScript. For example material like that in core/modules/parsers/base.js would appear to not be useful in the context of the $:/core that is built for the client. The function signature docs seem much more reasonable in comparison.

If the TS decorations are only required under Node.js it would be worth exploring ways to limit some of the bulk from affecting the client.

linonetwo · 2024-10-30T06:45:45Z

I was learnt from $:/core/modules/parsers/wikiparser/rules/wikirulebase.js. And it is not possible to define class in pure JSDoc, it need code in JS or TS. But What if I remove the

/*\
title: $:/core/modules/parsers/base.js
type: application/javascript
module-type: library
\*/

on the top, it will be ignored.

So there will no be a JS to be parsed (It is not executed, only parsed by JS engine).

And JSDoc can be removed by html-minifier-terser in user side. Or in Github Actions that release the empty edition.

Anything else to consider? Note that I add this to the publish.sh

# Build typings
npm i typescript
npx tsc

Jermolene · 2024-10-30T08:38:27Z

So there will no be a JS to be parsed (It is not executed, only parsed by JS engine).

Excellent, that's exactly what we need here. I wonder if it might be helpful to put these server side only files in their own subdirectory such as core/modules/server-only. Or perhaps it would be better to establish a naming convention such as foobar.server-side.js and keep the files in the logical subdirectories.

And JSDoc can be removed by html-minifier-terser in user side. Or in Github Actions that release the empty edition.

I am not sure that we should strip the method signature JSDoc comments. They can still be useful for debugging.

I guess we should check whether any client side dev tools understands TypeScript; if so, there might be a case for including the full JSDoc info.

Anything else to consider? Note that I add this to the publish.sh
# Build typings
npm i typescript
npx tsc

Hmmm. Perhaps I had misunderstood how intertwined JSDoc and TypeScript has become. I'm open to discussion but my de facto position is that TiddlyWiki 5 targets vanilla JS, and should not require any external tooling besides Node.js and itself.

When we started this discussion I had expected that we would explore the impact of adding JS comments to the source code, and that we would have tooling to generate documentation from those comments. That documentation would be the big benefit for users of TiddlyWiki.

As it has turned out, this PR brings in much more TypeScript than I had expected. For example, it appears that these new base classes don't really do anything to improve the documentation, they are only for the benefit of TypeScript tooling.

So, we should continue to regard this as an experiment – perhaps you could change the status to "Draft"?

I would like to continue to explore two areas:

The impact on the size of the core of adding the JSDoc comments, and the strategies we can use to reduce the burden (eg the technique discussed above)
Explore the options for producing documentation from the JSDoc comments

pmario · 2024-10-30T09:18:36Z

I think it does not profit any TypeScript tooling. It uses existing TS tooling to help VSCode with auto-complete. Which in turn helps us developers.

VSCode does understand this type of JSDoc and is able to show a popup with function parameter information. If you hover a function anywhere in the core code.

The main problem is. If it is done consistently for all TW core API functions the core size will increase significantly, without a build step, that removes that info again.

Since TW has its own build steps, it should be straight forward to remove JSDoc if we want to. eg:

tiddlywiki ./edtions/abc --strip-jsdoc --build index

That's possible without any 3rd party build tools.

Jermolene · 2024-10-30T09:34:24Z

Since TW has its own build steps, it should be straight forward to remove JSDoc if we want to. eg:

The trouble with that approach is that people using browser developer tools would lose all the comments. We cannot let the developer experience in the browser suffer because of the needs of VSCode developers on the server.

The main problem is. If it is done consistently for all TW core API functions the core size will increase significantly

I guess this is the heart of the quandary; regardless of the format used, adding comments will increase the size of the source code. Stripping them will deprive browser developer tools users of essential debugging information.

Perhaps a way to break the Gordian knot might be to introduce this change at the same time as compression for plugin contents. One would hope that even with extensive comments added the core plugin when compressed would still be smaller than the current core plugin.

linonetwo · 2024-10-30T09:53:56Z

Initially I only want to add JSDoc for AST related types, they are the hard part for plugin developers. Other wiki method's typing are side products, just to show what it can do for core developers.

We can only add JSDoc for complex methods, no need to cover all. If we are going to cover all, maybe we could write a script to remove all comment lines starts with @ to remove all JSDoc.

If you don't want publish.sh to install TS, I can remove it, and ask plugin developers (basically just me and oeyoews in codemirror6) to manually run tsc in node_modules, while this is not a good experience.
If publish.sh could run inside Github Action so it won't install TS on your computer, but now it isn't so I understand you don't want it to cause side effects.

using browser developer tools would lose all the comments

I don't understand this, usually we don't need to read comments in the browser. They can be read in a doc site, like https://tiddly-gittly.github.io/TW5-Typed/ generated from typings.

Jermolene · 2024-10-30T10:25:03Z

Hi @linonetwo I am very keen for TiddlyWiki to offer a first class experience for TypeScript/VSCode users, and I think complete JSDoc coverage would be a good long term goal for us. I think it's probably OK to use TS in scripts like publish.sh; the only hard requirement is that people using TiddlyWiki under Node.js shouldn't have to run npm install to get it working.

I don't understand this, usually we don't need to read comments in the browser. They can be read in a doc site, like https://tiddly-gittly.github.io/TW5-Typed/ generated from typings

The comments are useful when stepping through code, which is a popular technique for JS developers learning how TW works.

feat: add example

602e5ea

linonetwo mentioned this pull request Aug 20, 2024

[IDEA] Add JSDoc to AST #8532

Open

Jermolene reviewed Aug 20, 2024

View reviewed changes

chore: add ts file so tsc --noEmit works

270aeba

refactor: remove // @ts-check

492d812

linonetwo added 3 commits September 10, 2024 15:14

feat: Allow import('$:/core/modules/...') instead of `import('../..…

ac308e0

…/core/modules/...')`. Only works inside this project.

feat: make $tw.wiki globally available

b6f219a

feat: add an example jsdoc to $tw.wiki.getTiddlerText

a5feb38

pmario reviewed Sep 10, 2024

View reviewed changes

linonetwo added 2 commits September 10, 2024 18:44

fix: Declaration emit for this file requires using private name 'Widg…

c5304b4

…et'. An explicit type annotation may unblock declaration emit.

fix: duplication of types

43eff4e

pmario reviewed Sep 10, 2024

View reviewed changes

linonetwo added 8 commits September 12, 2024 11:47

docs: shorten JSDoc comment as requsted

cdceedd

refactor: move some type to base class

8f0d37e

feat: allow type in /generated to work

24279cc

feat: import base type

e3fc9b4

refactor: generate types to /types/core instead for simplicity

18517b6

feat: add ast type that wikiast is using

c56afea

feat: refine the ast type

06204b1

Update tw.d.ts

0c92f1b

linonetwo marked this pull request as ready for review September 14, 2024 12:38

linonetwo mentioned this pull request Oct 25, 2024

Planning for release of v5.3.6 #8680

Open

Jermolene reviewed Oct 29, 2024

View reviewed changes

linonetwo added 3 commits October 30, 2024 14:49

fix: ignore base file

562a308

Update wiki.js

3cc1849

Update npm-publish.sh

2eaadcd

linonetwo marked this pull request as draft October 30, 2024 09:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add JSDoc example #8533

feat: add JSDoc example #8533

linonetwo commented Aug 20, 2024

github-actions bot commented Aug 20, 2024

Jermolene left a comment

Jermolene Aug 20, 2024

linonetwo Aug 21, 2024

Jermolene Aug 21, 2024

linonetwo Aug 21, 2024 •

edited

Loading

Jermolene Aug 20, 2024

linonetwo Sep 10, 2024

linonetwo Sep 10, 2024 •

edited

Loading

pmario commented Sep 9, 2024

linonetwo commented Sep 10, 2024

linonetwo commented Sep 10, 2024

linonetwo commented Sep 10, 2024 •

edited

Loading

pmario Sep 10, 2024

pmario Sep 10, 2024 •

edited

Loading

linonetwo Sep 10, 2024

pmario Sep 10, 2024 •

edited

Loading

linonetwo Sep 10, 2024

pmario Sep 10, 2024

linonetwo Sep 12, 2024 •

edited

Loading

linonetwo commented Sep 10, 2024

pmario Sep 10, 2024

linonetwo Sep 12, 2024

linonetwo commented Sep 12, 2024

Jermolene left a comment

linonetwo commented Oct 30, 2024 •

edited

Loading

Jermolene commented Oct 30, 2024

pmario commented Oct 30, 2024

Jermolene commented Oct 30, 2024

linonetwo commented Oct 30, 2024 •

edited

Loading

Jermolene commented Oct 30, 2024

feat: add JSDoc example #8533

Are you sure you want to change the base?

feat: add JSDoc example #8533

Conversation

linonetwo commented Aug 20, 2024

github-actions bot commented Aug 20, 2024

Jermolene left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

linonetwo Aug 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

linonetwo Sep 10, 2024 • edited Loading

Choose a reason for hiding this comment

Without JSDoc:

With JSDoc:

pmario commented Sep 9, 2024

linonetwo commented Sep 10, 2024

linonetwo commented Sep 10, 2024

linonetwo commented Sep 10, 2024 • edited Loading

Choose a reason for hiding this comment

pmario Sep 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pmario Sep 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

linonetwo Sep 12, 2024 • edited Loading

Choose a reason for hiding this comment

linonetwo commented Sep 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

linonetwo commented Sep 12, 2024

1. tsconfig.json in plugin root

2. run tsc in node_modules/tiddlywiki/

3. enjoy the type

4. maintaining the type

Jermolene left a comment

Choose a reason for hiding this comment

linonetwo commented Oct 30, 2024 • edited Loading

Jermolene commented Oct 30, 2024

pmario commented Oct 30, 2024

Jermolene commented Oct 30, 2024

linonetwo commented Oct 30, 2024 • edited Loading

Jermolene commented Oct 30, 2024

linonetwo Aug 21, 2024 •

edited

Loading

linonetwo Sep 10, 2024 •

edited

Loading

linonetwo commented Sep 10, 2024 •

edited

Loading

pmario Sep 10, 2024 •

edited

Loading

pmario Sep 10, 2024 •

edited

Loading

linonetwo Sep 12, 2024 •

edited

Loading

2. run `tsc` in `node_modules/tiddlywiki/`

linonetwo commented Oct 30, 2024 •

edited

Loading

linonetwo commented Oct 30, 2024 •

edited

Loading