Skip to content

Conversation

@NathanTBeene
Copy link

The Issue

While attempting to use this module in a vscode extension, there was a path resolution bug where __dirname resolved to the base extension folder instead of the module itself, thus causing the notifications to not work correctly since the binaries (ntfytoast.exe, notifu.exe, and terminal-notifier) couldn't be found.

The Solution

Replaced path.resolve() and path.join() with require.resolve() for all binaries that used relative paths. With require.resolve(), it can correctly handle the relative path without the need for __dirname.

This should also help with the webpack, esbuild, and electron configurations as these should properly resolve without needing to set the electron configuration. (Though I didn't fix the __filename reference since it was unrelated to my issue.)

Changes

  • Modified Files:
    • notifiers/baloon.js
      • Changed to require.resolve()
      • Added logic to strip and add the .exe back to the filepath for the 64-bit suffix handling.
    • notifiers/notificationcenter.js
      • Changed to require.resolve()
    • notifiers/toaster.js
      • Changed to require.resolve()
      • Removed .exe concatenation since resolve includes the full extension.
  • New Test File:
    • test/module-resolution.js - A test suite of 14 tests that should cover:
      • Binary path resolution for all platforms
      • path consistency
      • Bundler and Webpack compatibility (verifies independence from the __dirname: true config)
      • Verification that old path.resolve(__dirname) pattern is removed
      • Extension handling for windows .exe files.

Testing

  • All 89 tests passed (75 existing + 14 new)

@BinaryServ BinaryServ added AC › Failed Autocheck failed to run through a complete cycle, requires investigation Type ◦ Pull Request Normal pull request labels Nov 21, 2025
@BinaryServ
Copy link
Collaborator


Automatic Self-Check - #38

The details of our automated scan for your pull request are listed below. If our scan detected errors, they must be corrected before this pull request will be advanced to the review stage:




About

This pull request includes the following information:

Category Value
Title Fix/module path resolution
Created 11.21.2025 10:18 PM UTC
ID #38
Author NathanTBeene
Repo node-toasted-notifier
Branch fix/module-path-resolution main
Added Files 1
Modified Files 4
Renamed Files 0
Copied Files 0
Deleted Files 0



📄 test/module-resolution.js

Caution

Errors must be fixed prior to a pull request being reviewed and accepted.
The file test/module-resolution.js contains the following errors:


fs import only available from Node.js runtime, this will throw errors for users running on mobile
fs import only available from Node.js runtime, this will throw errors for users running on mobile.




📄 notifiers/balloon.js

Note

The file notifiers/balloon.js contains no errors





📄 notifiers/notificationcenter.js

Note

The file notifiers/notificationcenter.js contains no errors





📄 notifiers/toaster.js

Note

The file notifiers/toaster.js contains no errors





This check was done automatically. Do NOT open a new PR for re-validation. Instead, to trigger this check again, make a change to your PR and wait a few minutes, or close and re-open it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AC › Failed Autocheck failed to run through a complete cycle, requires investigation Type ◦ Pull Request Normal pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants