Ignore HTTP imports in no-missing-import - #547
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17115f0f14
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| (this.moduleType === "data" || this.moduleType === "http") && | ||
| this.moduleStyle === "import" | ||
| ) { | ||
| return this.name |
There was a problem hiding this comment.
Avoid treating URL imports as file paths
For http(s) imports this returns the URL string as target.filePath, but other consumers of ImportTarget assume filePath is a real filesystem path. In n/no-unpublished-import, checkPublish calls toRelative(target.filePath) for targets with no moduleName; when a published file imports https://..., that URL is interpreted as a relative path (often outside the package, depending on cwd/package layout) and can be reported as "https://..." is not published. The no-missing rule only needs to suppress resolveError, so URL imports should be skipped without exposing them as filesystem paths to publish checks.
Useful? React with 👍 / 👎.
HTTP(S) specifiers were being classified through the package-import path in
ImportTarget, son/no-missing-importcould report them as missing files. The shared import-target utility now classifies them as URL imports before package matching and skips filesystem resolution forhttp:andhttps:imports.The rule run passed with 78 cases. The full rules run passed with 3059 passing and 3 pending, along with ESLint and TypeScript checks. Fixes #65