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

CommonJS globals permitted for ES module builds with no compiler error. #58658

Open
knightedcodemonkey opened this issue May 25, 2024 · 5 comments
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@knightedcodemonkey
Copy link

πŸ”Ž Search Terms

nodenext, module, __dirname

πŸ•— Version & Regression Information

Version 5.4.5

⏯ Playground Link

No response

πŸ’» Code

package.json

"type": "module"

tsconfig.json

{
  "compilerOptions": {
    "module": "NodeNext"
  },
  "include": ["src"]
}

file.ts

console.log(__dirname)

πŸ™ Actual behavior

No compiler error, but this causes a runtime error in Node:

ReferenceError: __dirname is not defined in ES module scope

πŸ™‚ Expected behavior

That the compiler issues an error similar to the inverse situation.

For example, when targeting CommonJS:

package.json

"type": "commonjs"

file.ts

console.log(import.meta.dirname)

The compiler issues the following error:

error TS1470: The 'import.meta' meta-property is not allowed in files which will build into CommonJS output.

Additional information about the issue

Perhaps there is a good reason for this that I'm not understanding, but I would think tsc should issue an error or warning for any syntax that would produce a runtime error.

Here is a more complete example: https://github.com/knightedcodemonkey/tsc-module-globals

  • npm install
  • npm run esm (note no compile error but the output causes a runtime error)
  • npm run cjs (note there is a compile error)
@knightedcodemonkey
Copy link
Author

knightedcodemonkey commented May 25, 2024

If when targeting "type": "module" and instead using console.log(globalThis.__dirname) there is still no compiler error and node does not throw the ReferenceError about __dirname since it is now a property of globalThis. However, if instead you do console.log(require.main), you still get no compile error, and node does throw the ReferenceError about require not being defined is ES module scope.

What is the recommended usage pattern here? I know people when writing ES modules often do something like this

const __dirname = dirname(fileURLToPath(import.meta.url))

@jakebailey
Copy link
Member

This is more or less expected as these are defined by @types/node as global, and there's no way to say "declare global but only if the current file is CJS".

@knightedcodemonkey
Copy link
Author

Thanks for the feedback.

I guess I'm wondering why the compiler doesn't introduce some heuristic to at least warn about using CJS globals when targeting ES module output based on the module setting and the type value from the package.json file?

@jakebailey
Copy link
Member

jakebailey commented May 30, 2024

TS does not have a concept of "warnings", though a "suggestion" diagnostic is close. It's also non-trivial because people frequently write ESM with an assumption about transpilation (but maybe want to typeof require != "undefined" etc), and we also consider any function named require to be require for purposes of module resolution.

There are lint rules that can be strict about this, if you fit within their guidelines.

@knightedcodemonkey
Copy link
Author

knightedcodemonkey commented Jun 4, 2024

Another interesting thing about the compiler behavior is

Given:

cjs.ts

exports.foo = 'bar';

package.json

"type": "module"

file.ts

import './cjs.js'

With module of NodeNext the compiler does not convert the exports.foo to a export const foo. Instead you get

exports.foo = 'bar';
export {};

Also, there is no compiler error, only a runtime error.

Is this somehow related to #56678?

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature labels Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

3 participants