WIP: Fix docs:verify in CI

[sidharthv96]

📑 Summary

Brief description about the content of your PR.

Resolves #

📏 Design Decisions

Describe the way your implementation works or what design decisions you made if applicable.

📋 Tasks

Make sure you

• 📖 have read the contribution guidelines
• 💻 have added unit/e2e tests (if appropriate)
• 🔖 targeted develop branch

[sidharthv96]

@weedySeaDragon docs:verify seems to be broken in CI.

I get this when running locally, but all tests pass in CI.

$ ts-node-esm src/docs.mts --verify
Transforming markdown files...
Transforming html files...
  File to be transformed: docs/index.html
Transforming all other files...
Changed files were transformed in src/docs but do not match the files in docs. Please run yarn docs:build after making changes to src/docs to update the docs directory with the transformed files.
error Command failed with exit code 1.

[weedySeaDragon]

When I run docs:verify it passes.
Looking back through the git commit history, I see that this commit 0605bce

resulted in a difference between src/docs/index.html and docs/index.html on line 27 (same line in each file):

docs/index.html line 27:

defer=""

but

src/docs/index.html line 27:

defer

When I checkout the branch revision 27e4024
then I get the same failure as you do with docs:verify.

When I go back a bit and checkout branch sidv/esbuild then docs:verify has no errors.

This makes me think that we should display the exact differences in files that causes docs:verify to fail. If you don't want to display that by default, we can have a --verbose flag that will display them.

[weedySeaDragon]

Wait -- now I'm seeing that error consistently, too.

AHA! Here's where the problem is: insertAutoGeneratedComment

The file from src/docs is converted into JSDOM, the "... automatically generated..." comment is inserted, and then it is converted back to a string with jsdom.serialize()

jsdom.serialize() is causing that subtle difference: it is inserting double quotes on line 27 (which are not in the original source file).

That serialized string is what is compared to the contents of the file in docs -- so they are considered the same.

I think a better approach would be something like:
srcContents = readfileSync(the file in the src directory)
docContents = readfileSync(the file in the docs directory)
docContents = docContents.replace(['... automatically generated comment...', '') // effectively removing the comment

and then compare srcContents and docContents.

(But I'm pretty tired right now.)
I can work on that tomorrow but I'm done here for the day.

[sidharthv96]

I think a better approach would be something like:
srcContents = readfileSync(the file in the src directory)
docContents = readfileSync(the file in the docs directory)
docContents = docContents.replace(['... automatically generated comment...', '') // effectively removing the comment
and then compare srcContents and docContents.

I don't think we have to do this. Because the comment is not the only transformation happening. For markdown files we are also changing the mermaid code blocks.

What ultimately matters should be transformed('src/doc') === 'doc'). So the defer="" is not an issue.

The reason I caught this error was this PR.

• Replace GoogleAnalytics with Plausible #3466

Eventhough I was the one who worked on the auto generation, I myself mistakenly committed to docs/index.html and it was subsequently merged with all tests passing. That change was not a minor formatting change, it had multiple lines removed and added.

What we should figure out is why that didn't fail in CI. Even this PR which has a line deleted in docs passes the lint docs:verify in CI whereas it fails in local.

I've already added a precommit hook, but that's not the ideal solution.

[weedySeaDragon]

This area (all of the hooks and scripts set up for the js project) is not anything I'm familiar with. I understand all the scripts, etc. called in package.json and github workflows, but the interaction/flow with the other ones you have set up via .husky and (it looks like to me) .lintstagedrc.json and .commitlintrc.json and probably more) isn't something I've worked with. (Ex: how is yarn:prepare called? )

I'm willing to help, but just not sure how I can.

[weedySeaDragon]

So I looked into how the scripts work together and put together the diagram below.

Take this with a giant grain of salt, given I am probably missing chunks of understanding....

Re precommit hooks: It doesn't look to me like .lintstagedrc.json actually calls docs.mts --verify.

The github workflow lint definitely calls it, though. (i'll see if I can look into that in a bit. )

flowchart LR
    prepare[[prepare]]-->huskyInstall[husky install]
    prepare-->build 
    clean[[clean]]--> rimraf["rimraf dist"]
    buildCode[[build:code]]-->esbuildCjs["node .esbuild/esbuild.cjs"]
     buildTypes[[build:types]]-->tsc["tsc -p ./tsconfig.json --emitDeclarationOnly"]
     buildWebpack[[build:webpack]]-->webpackProduction["webpack --mode production --progress --color"]
     buildWatch[[build:watch]]-->buildCode
     buildNew[[build:new]]-->buildCode
        buildNew-->buildTypes
     build[[build]]-->clean-->buildWebpack
     docsBuild[[docs:build]]-->docsMts["ts-node-esm src/docs.mts"]
     
     docsVerify[[docs:verify]]-->docsMtsVerify["ts-node-esm src/docs.mts --verify"]
    postbuild[[postbuild]]-->documentation[documentation ...]-->prettierWrite[prettier --write]-->docsBuild
    
     release[[release]]-->build
     lint[[lint]]-->eslint[eslint]-->prettierCheck[prettier --check]
     lintFix[[lint:fix]]-->eslintFix["eslint --fix --ignore-path .gitignore . "] -->prettierWrite
     e2eDepr[[e2e:depr]]-->lint-->jestE2e[jest e2e]
     cypress[[cypress\ncypress run]]
     cypressOpen[[cypress:open\cypress open]]
     e2e[[e2e]]--> startServerTest["start-server-and-test dev http://localhost:9000/ cypress"]
    e2e-upd[[e2e-upd]]-->lint-->jestE2e
     dev[[dev]]-->webpackServe["webpack serve --config ./.webpack/webpack.config.e2e.babel.js"]
     ci[[ci]]-->jestSrc["jest src/.*"]
     test[[test]]-->lint-->jestSrc
     testWatch[[test:watch]]--> jestWatch["jest --watch src"]
     prepublishOnly[[prepublishOnly]]-->build
     prepublishOnly[[prepublishOnly]]-->test   
    preCommit[["pre-commit\n git pre-commit hook\n (via .husky)"]]-->lintStaged
    lintStaged([<i>.lintstagedrc.json</i>])-->docsMtsGit["src/docs/**: ts-node-esm src/docs.mts --git"]-->docsMtsGit2["src/docs.mts: ts-node-esm src/docs.mts --git"]-->eslintManualFix[eslint --fix]-->prettierManualWrite[prettier --write]

Loading

[sidharthv96]

Damn that's a big chart!

It doesn't look to me like .lintstagedrc.json actually calls docs.mts --verify.

Yes, I did try adding a verify call if docs folder is edited. It works the first time you make changes to src/docs but is any of the other precommit hook fails, and you make a change to another src/docs file is changed, it will never pass again.

We could add more logic to it, but feels like there's a lot of complexity now itself.
So it'd be much better if the CI fails.
We'll anyway need the CI since pre commit hooks can be skipped/turned off.