Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor: Convert CommonJS to ECMAScript Modules (ESM) #1341

Open
wants to merge 44 commits into
base: main
Choose a base branch
from

Conversation

raosan
Copy link
Contributor

@raosan raosan commented Jan 6, 2025

Monika Pull Request (PR)

What feature/issue does this PR add

  1. Convert CommonJS to ECMAScript Modules (ESM) #1230

How did you implement / how did you fix it

  1. Fix import statement in almost every file
  2. Add additional config in .mocharc.json
  3. Fix implementation of unit test that use stub or spy

How to test

  1. run npm ci and npm run build -w packages/notification
  2. run npm run test and see the tests is run successfully like before
  3. run npm start and see that the apps is running ok

Notes

  1. Some test files is excluded temporarily in .mocharc.json because i am stuck to do stub/spy test that are using 3rd party library.
    Screenshot 2025-01-10 at 14 41 09

@lukman7788 lukman7788 self-requested a review January 16, 2025 07:53
Copy link
Contributor

@lukman7788 lukman7788 left a comment

Choose a reason for hiding this comment

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

LGTM

@syamsudotdev
Copy link
Contributor

syamsudotdev commented Jan 29, 2025

Monika will crash when assertion get triggered

Steps to reproduce

  1. gh pr checkout 1341
  2. npm ci
  3. npm run build -w packages/notification
  4. Create monika.yml below
probes:
  - id: 'http-1'
    name: 'status-401-test'
    interval: 1
    requests:
      - url: https://httpbin.org/status/401
        method: PATCH
    alerts:
      - assertion: response.status > 200
        message: HTTP Status is not 200
      - assertion: response.time > 2000
        message: Too sloow
notifications:
  - id: desktop
    type: desktop
  1. Run npm run start -- --config monika.yml -r 1
> @hyperjumptech/monika@1.21.2 start
> npm run prepack && ./bin/dev.js --config monika.yml -r 1


> @hyperjumptech/monika@1.21.2 prepack
> npm run clean && tsc -b && oclif manifest


> @hyperjumptech/monika@1.21.2 clean
> rm -rf coverage lib tsconfig.tsbuildinfo oclif.manifest.json

wrote manifest to /Users/nrsys/Projects/hyperjump/monika/oclif.manifest.json
Starting Monika. Probes: 1. Notifications: 1


INFO: Monika is running.
INFO: Using config file: /Users/nrsys/Projects/hyperjump/monika/monika.yml
WARN: 2025-01-29T16:11:22.473Z 1 id:http-1 Probe assertion failed. Will try again. Attempt (1) with incident threshold (5).
WARN: 2025-01-29T16:11:25.087Z 1 id:http-1 Probe assertion failed. Will try again. Attempt (2) with incident threshold (5).
WARN: 2025-01-29T16:11:27.523Z 1 id:http-1 Probe assertion failed. Will try again. Attempt (3) with incident threshold (5).
WARN: 2025-01-29T16:11:30.497Z 1 id:http-1 Probe assertion failed. Will try again. Attempt (4) with incident threshold (5).
INFO: 2025-01-29T16:11:39.436Z - Connected to STUN Server. Monika is running.
WARN: 2025-01-29T16:11:44.861Z 1 id:http-1 ASSERTION: response.status > 200, NOTIF: Service probably down
file:///Users/nrsys/Projects/hyperjump/monika/src/components/notification/alert-message.ts:53
}
                      ^
TypeError: Handlebars.compile is not a function
    at getExpectedMessage (file:///Users/nrsys/Projects/hyperjump/monika/src/components/notification/alert-message.ts:53:23)
    at getMessageForAlert (file:///Users/nrsys/Projects/hyperjump/monika/src/components/notification/alert-message.ts:85:29)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async sendAlerts (file:///Users/nrsys/Projects/hyperjump/monika/src/components/notification/index.ts:30:21)
    at async HTTPProber.sendNotification (file:///Users/nrsys/Projects/hyperjump/monika/src/components/probe/prober/index.ts:164:9)

Expectation

Monika will send notification HTTP Status is not 200

Platform Info

  • npm v10.2.4
  • node v20.11.0
  • MacOS 15.2

@raosan
Copy link
Contributor Author

raosan commented Jan 31, 2025

Monika will crash when assertion get triggered

Steps to reproduce

  1. gh pr checkout 1341
  2. npm ci
  3. npm run build -w packages/notification
  4. Create monika.yml below
    ...

Solved with the latest commit

Copy link
Contributor

@syamsudotdev syamsudotdev left a comment

Choose a reason for hiding this comment

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

Nice work!

@raosan
Copy link
Contributor Author

raosan commented Feb 3, 2025

postpone merge, waiting for:

  • updated documentation and CI about executable
  • manual test on 3rd party library that use import *

@syamsudotdev
Copy link
Contributor

manual test on 3rd party library that use import *
All used import are working as expected. Tested 3rd party libs with import * below

  • sqlite3 for SQLite store
  • nodemailer for SMTP notification
  • os for monika self update
  • unzipper for monika self update

@raosan

@sapiderman
Copy link
Contributor

Please check with symon mode, I'm getting the error:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants