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

nodenext doesn't pick up on addition of type field to package.json #50086

Closed
DanielRosenwasser opened this issue Jul 28, 2022 · 3 comments · Fixed by #50098
Closed

nodenext doesn't pick up on addition of type field to package.json #50086

DanielRosenwasser opened this issue Jul 28, 2022 · 3 comments · Fixed by #50098
Assignees
Labels
Bug A bug in TypeScript Domain: Node ESM Like ES Modules, but specific to Node.js support (cts, cts, mjs, mts) Fix Available A PR has been opened for this issue

Comments

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Jul 28, 2022

Here's a quick way you can clone this repro locally

https://github.com/DanielRosenwasser/daniel-ts-repros/
cd daniel-ts-repros
git checkout nodenext-package-watch-bug

// src/fileA.ts
import { foo } from "./fileB.mjs";

foo();
// src/fileB.mts
export function foo() {
}
// src/tsconfig.json
{
  "compilerOptions": {
    "target": "es2016",
    "module": "Node16",
    "outDir": "../out"
  }
}
// package.json

{
  "name": "nodenext-package-watch-bug",
  "version": "1.0.0",
  "description": "Repro",
  "author": "Daniel"
}

Currently this produces the following result in the editor or in --watch mode:

Module './fileB.mjs' cannot be imported using this construct. The specifier only resolves to an ES module, which cannot be imported synchronously. Use dynamic import instead.ts

If you add a "type": "module" to the package.json, then the error will not go away until you restart tsc or TS Server.

-  "author": "Daniel"
+  "author": "Daniel",
+  "type": "module"
@DanielRosenwasser DanielRosenwasser added Bug A bug in TypeScript Domain: Node ESM Like ES Modules, but specific to Node.js support (cts, cts, mjs, mts) labels Jul 28, 2022
DanielRosenwasser added a commit to DanielRosenwasser/daniel-ts-repros that referenced this issue Jul 28, 2022
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 4.8.1 milestone Jul 28, 2022
@sheetalkamat
Copy link
Member

Looks like package json is read for file format rather than for resolution .. which is why it’s not watched ..

@fatcerberus
Copy link

Possible root cause of #50058?

@sheetalkamat
Copy link
Member

Working on fixing this, but its not going to be 2 line fix..

sheetalkamat added a commit that referenced this issue Jul 30, 2022
…e. Handle implied node format in document registry

Fixes #50086
@typescript-bot typescript-bot added the Fix Available A PR has been opened for this issue label Jul 30, 2022
sheetalkamat added a commit that referenced this issue Aug 1, 2022
…dits (#50098)

* Add test where module resolution cache is not local and hence doesnt report errors in watch mode

* Ensure module resolution cache is passed through in watch mode

* Remove unnecessary setting of impliedFormat which should anyways be done as part of create source file

* Add test for packge.json changing and modifying implied format

* Distinguish between package.json watch and affecting file location watch

* Pass in failed lookup and affected file locations for source file's implied format
Also stop creating options if we already have them

* Add diagnostic for explaining file's implied format if based on package.json

* Watch implied format dependencies for modules and schedule update on change

* For program if implied node format doesnt match create new source file. Handle implied node format in document registry
Fixes #50086

* Modify tests to show package.json being watched irrespective of folder its in

* Check file path if it can be watched before watching package.json file

* Because we are watching package.json files and failed lookups its safe to invalidate package json entries instead of clearing them out everytime program is created

* Remove todos

* Fix the incorrect merge

* Pickup PackageJsonInfo renames from #50088

* Rename
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: Node ESM Like ES Modules, but specific to Node.js support (cts, cts, mjs, mts) Fix Available A PR has been opened for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants