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

Improve error messages used by esm-resolver #1373

Merged
merged 2 commits into from Jun 16, 2021

Conversation

tars0x9752
Copy link
Contributor

@tars0x9752 tars0x9752 commented Jun 13, 2021

Implements #1357 EDIT Closes #1357 (github needs this syntax to link the issues)

Make stub implementation of ERR_MODULE_NOT_FOUND message a little nicer.

@codecov
Copy link

codecov bot commented Jun 14, 2021

Codecov Report

Merging #1373 (b9fd206) into main (6266ae2) will decrease coverage by 0.13%.
The diff coverage is 25.00%.

Impacted Files Coverage Δ
dist-raw/node-errors.js 50.00% <25.00%> (-16.67%) ⬇️

@cspotcode
Copy link
Collaborator

Do those other error types always receive string arguments? In other words, is there a chance we'll end up putting [object Object] or something else cryptic in an error message?

@tars0x9752
Copy link
Contributor Author

I left those other error types original behavior as it is. So actually, I haven't checked that far. Well, I'll check it out anyway. 👍

@cspotcode
Copy link
Collaborator

Oh, they are unchanged. My mistake, I forgot that they already behaved that way.

@cspotcode
Copy link
Collaborator

@tars0x9752 this looks good to me. I see we have a gap in our test coverage here. Did you test informally, and did the error message look correct when ts-node threw it? Since we do not already have an automated test for this, I don't think you need to add one.

Otherwise this looks good to merge.

@tars0x9752
Copy link
Contributor Author

tars0x9752 commented Jun 14, 2021

Thanks. :)

I did the test and here's what it looks like. (And that's what I expected.)

$ node --loader=ts-node/esm foo.ts
(node:4352) ExperimentalWarning: --experimental-loader is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)

/home/naoki/workspace/ts-node/dist-raw/node-esm-resolve-implementation.js:383
    throw new ERR_MODULE_NOT_FOUND(
          ^
Error: Cannot find module '/home/naoki/workspace/ts-node/foo.ts' imported from /home/naoki/workspace/ts-node/
    at finalizeResolution (/home/naoki/workspace/ts-node/dist-raw/node-esm-resolve-implementation.js:383:11)
    at moduleResolve (/home/naoki/workspace/ts-node/dist-raw/node-esm-resolve-implementation.js:818:10)
    at Object.defaultResolve (/home/naoki/workspace/ts-node/dist-raw/node-esm-resolve-implementation.js:929:11)
    at /home/naoki/workspace/ts-node/src/esm.ts:67:38
    at Generator.next (<anonymous>)
    at /home/naoki/workspace/ts-node/dist/esm.js:8:71
    at new Promise (<anonymous>)
    at __awaiter (/home/naoki/workspace/ts-node/dist/esm.js:4:12)
    at resolve (/home/naoki/workspace/ts-node/dist/esm.js:31:16)
    at Loader.resolve (internal/modules/esm/loader.js:88:40)

@tars0x9752
Copy link
Contributor Author

tars0x9752 commented Jun 14, 2021

BTW, I did a quick search and came up with the following. (not 100% sure but It'll help some) And I found some cases to receive some none-string values. so I'll leave it here for future reference.

  • ERR_INPUT_TYPE_NOT_ALLOWED
    • doesn't receive any arguments.
  • ERR_INVALID_ARG_VALUE
    • receives an unknown value.
    • If the value of conditions, which supposed to be a string array, ISN'T string array, It receives the conditions as args.
  • ERR_INVALID_MODULE_SPECIFIER
    • string only
  • ERR_INVALID_PACKAGE_CONFIG
    • Almost string only. Possibly receives a boolean value false, But It's not a huge problem I think.
  • ERR_INVALID_PACKAGE_TARGET
    • Almost string only. Possibly receives a boolean value false.
  • ERR_MANIFEST_DEPENDENCY_MISSING
    • string only
  • ERR_MODULE_NOT_FOUND
    • string only
  • ERR_PACKAGE_IMPORT_NOT_DEFINED
    • Almost string only. Not sure but possibly receives a some kind of falsy value if packageJSONUrl is falsy. It looks like packageJSONUrl can be falsy only when it's undefined I guess. (not sure I may be wrong)
  • ERR_PACKAGE_PATH_NOT_EXPORTED
    • string only
  • ERR_UNSUPPORTED_DIR_IMPORT
    • string only
  • ERR_UNSUPPORTED_ESM_URL_SCHEME
    • Receives URL object. Well, I guess it might be cryptic. 🤔

@cspotcode
Copy link
Collaborator

Excellent, thank you. I'll get this merged soon and we'll give you a shout-out in the release notes.

@cspotcode cspotcode merged commit 5643ad6 into TypeStrong:main Jun 16, 2021
@cspotcode cspotcode added this to the 10.1.0 milestone Jul 9, 2021
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.

Friendlier error when the .ts file is not found
2 participants