Bump dateformat from 3.0.3 to 4.5.1. Closes #1324.

[rcyeh]

Bump dateformat from 3.0.3 to 5.0.3. Closes #1324

[riccardoferretti]

There seems to be an issue with the import

[rcyeh]

I'm sorry! I will try to work on that Friday night.

[rcyeh]

I'm running into some other problems. I'd like to figure this out in case I can contribute something in the future.

My environment is Linux penguin 6.1.60-08594-g03a802b9a072 on ChromeOS. The nodejs and yarn in the default repository were too old, so I installed nodejs 20.11.0, npm 10.3.0, yarn 4.0.2. My process is: yarn install; yarn build; yarn test.

After bumping the dateformat version, I first encountered an error:

jest encountered an unexpected token: export

which I interpreted to mean that the Typescript dateformat needed to be compiled to Javascript. So I added a section to the top-level package.json file:

• "jest": {
• "transformIgnorePatterns": [

•  "node_modules/dateformat"
  

• ]
• },

Next, yarn test obtained:

TypeError: Class extends value undefined is not a constructor or null "TestEnvironment"

so I did npm install jest-environment-node

Now, yarn test obtains:

TypeError: Cannot read properties of undefined (reading 'testEnvironmentOptions')

Have you seen any of these before?

Thanks, Richard

[riccardoferretti]

Hi Richard, the first thing I would advice is checking if you can run the tests in your environment before any changes (basically against the master branch). If those tests pass, then see if they pass with the changes.

Given that the error is also happening in CI I believe the issue is not strictly due to your environment.

Another avenue could be to make sure the node version required by the library. We use node 18 (looking at the engine configuration I have a feeling we should be more restrictive with the version) To manage node versions I would suggest looking into nvm.

[rcyeh]

Thank you! My problems reproduced at master. To correct this, I needed to npm install jest-environment-jsdom. After that, the tests ran!

Then I had some test failures because I had some foam preferences in ~/.config/Code/User/settings.json, which were used by the test version of vscode. Fixed my moving them to the .vscode/settings.json in my notes repository.

Finally, there were some errors where jest could not handle the ECMAScript version of dateformat.js (5.0.3), but could use the latest version before the conversion to ECMAScript (4.5.1). I tried the jest advice to transform dateformat (exclude from transformIgnorePatterns), but failed to get that to work.

Now all the tests pass.

So I will update this pull request with the package.json specifying jest-environment-jsdom and the packages/foam-vscode/package.json specifying dateformat 4.5.1. (^4.5.1 results in 4.6.3, which is ECMAScript). Should I also include the updated yarn.lock file in the pull request?

[rcyeh]

I synced with origin/master and just took the yarn.lock from the repository. Tests continue to pass.

[rcyeh]

OK finally! Cleaned up the commit. To explain the added dependencies: husky is needed to run the git commit hooks. jest, jest-environment-node, and jest-environment-jsdom are needed to run tests.

[rcyeh]

Hi Riccardo, thank you for helping me get my environment working. I updated the pull request last week. Tests pass.

I was able to use node 20.11 as well.

[riccardoferretti]

This looks good. Quick q: is there a reason for a upgrading jest?

[rcyeh]

No reason, that version of jest was just the one that npm install picked up. Should I set the earliest version that doesn’t have the problem? I will also search this repo to see if there is a specific version requirement that I can copy.

[riccardoferretti]

The jest dependency

"jest": "^29.6.2",

is defined in the package.json of foam-vscode.

I wonder if things would work even if you removed the new dependency you introduced. And if that's not the case, I would update the dependency in the package, where the old one is defined.

[rcyeh]

Thanks for the advice! When I run “yarn install; yarn build; yarn test” should I be at the top level of foam or one directory in, at foam-vscode?

[riccardoferretti]

I mostly do it from the top level, but either should work

[rcyeh]

OK thank you. To explain the current pull request, there are two changes:

• update dateformat requirement
• add jest to top-level package.json devDependencies section

I find that in my setup, if I:

0. regardless of whether I have changed the requirement for dateformat and
1. do not have "jest" = "^29.6.2" in the top-level package.json devDependencies and
2. rm -rf node_modules packages/foam-vscode/node_modules (not sure why that second one was there; but this step simulates a clean state)
3. yarn install; yarn build; yarn test

then the initial tests pass, but then all of the vscode tests fail, with the following error (path and middle of stack trace differ for each test) as if TestEnvironment were unable to be resolved. I don't remember if this is the same error that was reported in the original attempt to merge this pull request with the CI. Do you think it is worth further investigating how my environment differs from yours?

FAIL src/dated-notes.spec.ts ● Test suite failed to run

TypeError: Class extends value undefined is not a constructor or null

  2 | const vscode = require('vscode');
  3 |
> 4 | class VscodeEnvironment extends TestEnvironment {
    |                                 ^
  5 |   async setup() {
  6 |     await super.setup();
  7 |     this.global.vscode = vscode;

  at Object.<anonymous> (src/test/support/vscode-environment.js:4:33)
  at Module._compile (../../node_modules/pirates/lib/index.js:136:24)
  at Object.newLoader (../../node_modules/pirates/lib/index.js:141:7)
  at Function.r._load

(.vscode-test/vscode-linux-x64-1.70.0/VSCode-linux-x64/resources/app/out/vs/workbench/api/node/extensionHostProcess.js:111:14538) at Function.b._load (.vscode-test/vscode-linux-x64-1.70.0/VSCode-linux-x64/resources/app/out/vs/workbench/api/node/extensionHostProcess.js:106:62507) at Function.v._load (.vscode-test/vscode-linux-x64-1.70.0/VSCode-linux-x64/resources/app/out/vs/workbench/api/node/extensionHostProcess.js:106:61875) at ScriptTransformer.requireAndTranspileModule @./transform/build/ScriptTransformer.js:892:66) at async TestScheduler.scheduleTests @./core/build/TestScheduler.js:333:13) at async runJest @./core/build/runJest.js:404:19) at async _run10000 @./core/build/cli/index.js:320:7) at async runCLI @.***/core/build/cli/index.js:173:3) at async /home/r/github/foam/packages/foam-vscode/out/test/suite.js:50:33

[riccardoferretti]

Thanks for the explanation, I did some local testing and was able to reproduce the problem. I also managed to fix it locally by updating in foam-vscode/package.json:

• jest to version ^29.7.0
• @types/jest to version ^29.5.12

(and removed jest from the top level package.json)

Can you validate this set up work for you as well? Happy to then merge the PR

[rcyeh]

Thanks! I tried to do the same:

• remove jest from devDependencies at top-level package.json (same as most recent force push to this PR)
• update packages/foam-vscode/package.json by adjusting jest and @types/jest versions as you describe

and unfortunately, after rm -rf node_modules; yarn install; yarn build; yarn test; I still get the condition (all unit tests pass, all VS Code test suites fail with TestEnvironment (TypeError: Class extends value undefined is not a constructor or null) error) as above.

Here is what npm ls on my system shows after those initial steps - jest in packages/foam-vscode, but not at top level. If I add jest to devDependencies as you suggested earlier, then the same sequence with yarn install; npm ls; shows jest also at the top level, and then the tests all pass.

r@penguin:~/github/foam$ rm -rf node_modules/
r@penguin:~/github/foam$ npm ls
npm ERR! code ELSPROBLEMS
npm ERR! missing: all-contributors-cli@^6.16.1, required by [email protected]
npm ERR! missing: foam-vscode@file:/home/r/github/foam/packages/foam-vscode, required by [email protected]
npm ERR! missing: lerna@^6.4.1, required by [email protected]
[email protected] /home/r/github/foam
├── UNMET DEPENDENCY all-contributors-cli@^6.16.1
├── UNMET DEPENDENCY foam-vscode@file:/home/r/github/foam/packages/foam-vscode
└── UNMET DEPENDENCY lerna@^6.4.1


npm ERR! A complete log of this run can be found in: /home/r/.npm/_logs/2024-02-14T12_53_34_954Z-debug-0.log
r@penguin:~/github/foam$ yarn install
➤ YN0000: · Yarn 4.0.2
➤ YN0000: ┌ Resolution step
➤ YN0000: └ Completed in 0s 304ms
➤ YN0000: ┌ Fetch step
➤ YN0000: └ Completed in 1s 70ms
➤ YN0000: ┌ Link step
➤ YN0007: │ esbuild@npm:0.17.7 must be built because it never has been before or the last one failed
➤ YN0007: │ husky@npm:4.3.8 must be built because it never has been before or the last one failed
➤ YN0007: │ @parcel/watcher@npm:2.0.4 must be built because it never has been before or the last one failed
➤ YN0007: │ nx@npm:15.6.3 [3443f] must be built because it never has been before or the last one failed
➤ YN0000: └ Completed in 6s 447ms
➤ YN0000: · Done in 8s 20ms
r@penguin:~/github/foam$ npm ls
[email protected] /home/r/github/foam
├── @isaacs/[email protected] extraneous
├── @npmcli/[email protected] extraneous
├── @pkgjs/[email protected] extraneous
├── [email protected]
├── [email protected] extraneous
├── [email protected] extraneous
├─┬ [email protected] -> ./packages/foam-vscode
│ ├── @types/[email protected]
│ ├── @types/[email protected]
│ ├── @types/[email protected]
│ ├── @types/[email protected]
│ ├── @types/[email protected]
│ ├── @types/[email protected]
│ ├── @types/[email protected]
│ ├── @types/[email protected]
│ ├── @types/[email protected]
│ ├── @typescript-eslint/[email protected]
│ ├── @typescript-eslint/[email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ └── [email protected]
├── [email protected] extraneous
├── [email protected] extraneous
├── [email protected] extraneous
├── [email protected] extraneous
├── [email protected]
├── [email protected] extraneous
├── [email protected] extraneous
├── [email protected] extraneous
├── string-width-cjs@npm:[email protected] extraneous
├── strip-ansi-cjs@npm:[email protected] extraneous
└── wrap-ansi-cjs@npm:[email protected] extraneous

(Edit: it appears that my email comment was redacted. Now pasted the actual output of npm ls.)

[riccardoferretti]

Mmmm.. can you try to do a yarn clean as well before the build? Odd what's happening.

I am not strongly opposed to moving the jest dependency at the top, but in that case I would remove it from the package (or at least have matching versions)

[rcyeh]

Thank you! yarn clean was what I needed! With the dateformat dependency update, there was a corresponding small change in yarn.lock, so I added that to the change. The tests all work now, without any change to the top-level (or any other) package.json.

I tried to delete the dateformat@^3.0.0 section from yarn.lock; but yarn install put it back.