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.

Sign up for GitHub

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

Jump to bottom

fix(check): npm resolution errors to tsc diagnostics #28174

Merged
merged 2 commits into from
Feb 18, 2025
Merged

fix(check): npm resolution errors to tsc diagnostics #28174

merged 2 commits into from
Feb 18, 2025

Conversation

dsherret
Copy link
Member

Closes #27188

@dsherret dsherret requested a review from bartlomieju February 18, 2025 19:31
dsherret
dsherret commented Feb 18, 2025
View reviewed changes
tests/specs/node/cjs_analysis_multiple_errors/main.out
@@ -1,3 +1,3 @@
[# Since this package has multiple cjs export errors and the resolution is done in ]
[# parallel, we want to ensure the output is deterministic when there's multiple errors]
error: [ERR_MODULE_NOT_FOUND] Cannot find module "[WILDLINE]not_exists.js" imported from "[WILDLINE]a.js"
error: [ERR_MODULE_NOT_FOUND] Cannot find module '[WILDLINE]not_exists.js' imported from '[WILDLINE]a.js'
Copy link
Member Author

Choose a reason for hiding this comment

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

Node uses single quotes.

bartlomieju
bartlomieju approved these changes Feb 18, 2025
View reviewed changes
Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

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

LGTM

@dsherret dsherret enabled auto-merge (squash) February 18, 2025 20:05
@dsherret dsherret merged commit f62fc9e into denoland:main Feb 18, 2025
18 checks passed
@invalidred
Copy link

invalidred commented Feb 28, 2025 •
edited
Loading

Hi Team,

This change seems to have been release in 2.2.0 however the original issue still remains as documented here #27188

I'm still noticing the following error when I run it on deno 2.2.2 today.

error: Error: Unable to load /Users/abdulkhan/github/asi/main/app/node_modules/.deno/@[email protected]/node_modules/@mapbox/search-js-web/src/types/custom_elements.d.ts: No such file or directory (os error 2)
    at Object.getSourceFile (ext:deno_cli_tsc/97_ts_host.js:472:26)
    at findSourceFileWorker (ext:deno_cli_tsc/00_typescript.js:127610:23)
    at findSourceFile (ext:deno_cli_tsc/00_typescript.js:127526:20)
    at ext:deno_cli_tsc/00_typescript.js:127475:22
    at getSourceFileFromReferenceWorker (ext:deno_cli_tsc/00_typescript.js:127446:26)
    at processSourceFile (ext:deno_cli_tsc/00_typescript.js:127473:5)
    at ext:deno_cli_tsc/00_typescript.js:127767:7
    at forEach (ext:deno_cli_tsc/00_typescript.js:2297:22)
    at processReferencedFiles (ext:deno_cli_tsc/00_typescript.js:127766:5)
    at findSourceFileWorker (ext:deno_cli_tsc/00_typescript.js:127669:9)
➜  app git:(akhan/adds-booking-id) ✗ deno check frontend/routes

Is there something I might not be doing right?

@dsherret dsherret deleted the fix_npm_resolution_errors_tsc_diagnostics2 branch February 28, 2025 00:23
@dsherret
Copy link
Member Author

Huh, you're right. I guess it only fixed it for certain scenarios. Will reopen.

This was referenced Feb 28, 2025
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bartlomieju bartlomieju bartlomieju approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Surface not found modules as diagnostics instead of erroring and failing to load
3 participants
@dsherret @invalidred @bartlomieju