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

Merged
merged 48 commits into from
Mar 7, 2025

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

@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

@raosan
Copy link
Contributor Author

raosan commented Mar 7, 2025

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

image

solved in the latest commit

Copy link
Contributor

@sapiderman sapiderman left a comment

Choose a reason for hiding this comment

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

latest updates worked for me. Nice! 👍🏽 👍🏽 👍🏽

@raosan raosan merged commit 702f7c0 into hyperjumptech:main Mar 7, 2025
7 checks passed
Copy link

codecov bot commented Mar 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base (6a29470) to head (afaed07).
Report is 41 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #1341       +/-   ##
==========================================
- Coverage   62.51%       0   -62.52%     
==========================================
  Files         112       0      -112     
  Lines        3391       0     -3391     
  Branches      591       0      -591     
==========================================
- Hits         2120       0     -2120     
+ Misses       1079       0     -1079     
+ Partials      192       0      -192     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@raosan raosan linked an issue Mar 7, 2025 that may be closed by this pull request
@raosan raosan deleted the feat/migrate-esm branch March 7, 2025 08:08
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.

Convert CommonJS to ECMAScript Modules (ESM)
5 participants