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

Remove declares in sys, core, evaluatorImpl #53176

Merged
merged 4 commits into from
Mar 10, 2023
Merged

Remove declares in sys, core, evaluatorImpl #53176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8f9ecbc
Remove declares in sys.ts, fix code
jakebailey Mar 9, 2023
8aea35d
Remove declare in core.ts
jakebailey Mar 9, 2023
c470b80
Remove declare in evaluatorImpl
jakebailey Mar 9, 2023
3d20625
Merge branch 'main' into remove-random-declares
jakebailey Mar 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions src/compiler/core.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2869,8 +2869,6 @@ function trimEndImpl(s: string) {
return s.slice(0, end + 1);
}

declare const process: any;

/** @internal */
export function isNodeLikeSystem(): boolean {
// This is defined here rather than in sys.ts to prevent a cycle from its
Expand All @@ -2880,7 +2878,7 @@ export function isNodeLikeSystem(): boolean {
// when bundled using esbuild, this function will be rewritten to `__require`
// and definitely exist.
return typeof process !== "undefined"
&& process.nextTick
&& !process.browser
&& !!process.nextTick
&& !(process as any).browser
&& typeof module === "object";
}
15 changes: 5 additions & 10 deletions src/compiler/sys.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1443,12 +1443,6 @@ interface DirectoryWatcher extends FileWatcher {
referenceCount: number;
}

declare const require: any;
declare const process: any;
declare const global: any;
declare const __filename: string;
declare const __dirname: string;

// TODO: GH#18217 this is used as if it's certainly defined in many places.
export let sys: System = (() => {
// NodeJS detects "\uFEFF" at the start of the string and *replaces* it with the actual
Expand Down Expand Up @@ -1507,7 +1501,7 @@ export let sys: System = (() => {
getAccessibleSortedChildDirectories: path => getAccessibleFileSystemEntries(path).directories,
realpath,
tscWatchFile: process.env.TSC_WATCHFILE,
useNonPollingWatchers: process.env.TSC_NONPOLLING_WATCHER,
useNonPollingWatchers: !!process.env.TSC_NONPOLLING_WATCHER,
Copy link
Member Author

Choose a reason for hiding this comment

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

This one is really suspicious; should we be parsing this rather than checking for truthiness? I made it !! to match current behavior, but, it seems unexpected.

Copy link
Member Author

Choose a reason for hiding this comment

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

@sheetalkamat Do you have any advice for this particular place? Is this "fine" or was it intended that we let you set TSC_NONPOLLING_WATCHER=false or TSC_NONPOLLING_WATCHER=0 or TSC_NONPOLLING_WATCHER=off or similar?

Copy link
Member

Choose a reason for hiding this comment

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

I think its fine to fix it this way. This is from way back... given we have watchOptions now i dont think it matters. We probably should deprecate these environment variable options.

Copy link
Member Author

Choose a reason for hiding this comment

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

Great, thanks!

The main benefit of the environment variables is being able to set them without modifying tsconfig, which is helpful if you're on a system where watch mode is broken and you don't want to commit a tsconfig change. (e.g. #52876 #52535)

tscWatchDirectory: process.env.TSC_WATCHDIRECTORY,
inodeWatching: isLinuxOrMacOs,
sysLog,
Expand Down Expand Up @@ -1584,7 +1578,7 @@ export let sys: System = (() => {
disableCPUProfiler,
cpuProfilingEnabled: () => !!activeSession || contains(process.execArgv, "--cpu-prof") || contains(process.execArgv, "--prof"),
realpath,
debugMode: !!process.env.NODE_INSPECTOR_IPC || !!process.env.VSCODE_INSPECTOR_OPTIONS || some(process.execArgv as string[], arg => /^--(inspect|debug)(-brk)?(=\d+)?$/i.test(arg)),
debugMode: !!process.env.NODE_INSPECTOR_IPC || !!process.env.VSCODE_INSPECTOR_OPTIONS || some(process.execArgv, arg => /^--(inspect|debug)(-brk)?(=\d+)?$/i.test(arg)),
tryEnableSourceMapsForHost() {
try {
(require("source-map-support") as typeof import("source-map-support")).install();
Expand All @@ -1599,8 +1593,9 @@ export let sys: System = (() => {
process.stdout.write("\x1Bc");
},
setBlocking: () => {
if (process.stdout && process.stdout._handle && process.stdout._handle.setBlocking) {
process.stdout._handle.setBlocking(true);
const handle = (process.stdout as any)?._handle as { setBlocking?: (value: boolean) => void };
if (handle && handle.setBlocking) {
handle.setBlocking(true);
}
},
bufferFrom,
Expand Down
2 changes: 0 additions & 2 deletions src/harness/evaluatorImpl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import * as ts from "./_namespaces/ts";
import * as vfs from "./_namespaces/vfs";
import * as vpath from "./_namespaces/vpath";

declare let Symbol: SymbolConstructor;

const sourceFile = vpath.combine(vfs.srcFolder, "source.ts");
const sourceFileJs = vpath.combine(vfs.srcFolder, "source.js");

Expand Down