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

BUG: (edge case) Typescript 4.5+ moduleResolution GlobalScope Introduce var global inside ESM Context edge case #47237

Closed
frank-dspeed opened this issue Dec 24, 2021 · 1 comment · Fixed by #47495
Assignees
Labels
Committed The team has roadmapped this issue ES Next New featurers for ECMAScript (a.k.a. ESNext) feature-request A request for a new feature Fix Available A PR has been opened for this issue

Comments

@frank-dspeed
Copy link

frank-dspeed commented Dec 24, 2021

Bug Report

global scope If your file hasn't any import or export line, this file would be executed in global scope that all declaration in it are visible outside this file.

This is a hard edge case as there is really no way to tell if that file will run in CJS or ESM Context When there is no additional package.json or if it is a .ts and not a .cts or mts

🔎 Search Terms

Typescript 4.5+ moduleResolution GlobalScope Introduce var global inside ESM Context

🕗 Version & Regression Information

  • This is a crash
  • This changed between versions ______ and _______
  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about _________
  • I was unable to test this on prior versions because _______

⏯ Playground Link

Playground link with relevant code

💻 Code

// inside CJS Context
// Your Code runs in the Global Context
var me = 'hi'
globalThis.me === me // true
// inside ESM Context
{
  // Your Code is Wrapped into its own scope with access to the globalThis
  var me = 'hi'
  globalThis.me === me // false
}

🙁 Actual behavior

.mts and .ts files introduce none existing globals when the target Environment is "ECMAScript Module Context"

// xx.ts or xx.mts

// without declare keyword
var age: number
var magicGlobalString = 'It is Magic'

// other.ts

globalThis.age = 18 // no error
globalThis.magicGlobalString = 'NoError' // no error

That introduces now the following edge case TypeScript Assumes that .ts files without import or require get executed in the globalThis Context and with CJS that is correct! But Running the Same code in Module Context will not Introduce a Global Var on the globalThis Context.

🙂 Expected behavior

at last for .mts it should not introduce a new global. looking into package,json does not make clear if the target will be esm or cjs for the .ts extension it only shows the current development settings before compilation.

maybe the tsconfig target setting can be used i did not explore that fully but my first experiments show that a single target is well and should be keept while i see the need to define a full set of targets to process the input files right.

so the dev environment needs the be target environment aware while working with development files.

This is possible with the current target setting but many people develop for diffrent targets

/esm-target/
  tsconfig.json <- with target ESNext
/cjs-target/
  tsconfig.json <- with target ES2015
/src <- es6 + ts + cjs the problem is now the input files from src need to get handled differently in the IDE and Language Service based on the fact if it is compiled with the cjs-target or esm-target config
/tsconfig.json <- main config that gets extended by *-target/tsconfig.json

and the most best would be if we could get errors in the language server based on all existing tsconfig files and the target property

Related Issues

@frank-dspeed frank-dspeed changed the title Typescript 4.5+ moduleResolution GlobalScope Introduce var global inside ESM Context edge case BUG: (edge case) Typescript 4.5+ moduleResolution GlobalScope Introduce var global inside ESM Context edge case Dec 24, 2021
@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Jan 5, 2022
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 4.7.0 milestone Jan 5, 2022
@weswigham weswigham added Committed The team has roadmapped this issue ES Next New featurers for ECMAScript (a.k.a. ESNext) feature-request A request for a new feature and removed Needs Investigation This issue needs a team member to investigate its status. labels Feb 10, 2022
@weswigham
Copy link
Member

#47495 should fix this. With it, by default, under nodenext or node12 resolution rules, all esm files will be locked to modules (regardless of syntax used), while for other scenarios, moduleDetection: 'force' can be set to force us to load all files as modules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Committed The team has roadmapped this issue ES Next New featurers for ECMAScript (a.k.a. ESNext) feature-request A request for a new feature 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