Provide flag for projects to ban compile-time side-effects in root files

gluxon commented 3 years ago

Search Terms

pure
side-effects
elision
global-modifying

Suggestion

A new tsconfig flag should be added that bans compile-time side-effects in project root files. This idea was originally proposed by @andrewbranch in response to an external pull request. The flag name can be discussed further, but examples might be noTypingSideEffects, noGlobalModifyingModules, noCompileTimeSideEffects, pure etc. The phrase "compile-time side-effects" refers to anything that can affect type-checking dependent source files outside of the standard export syntax.

The TypeScript features that allow source files to modify typing information outside of their file scope include the following:

With this flag enabled, an error should appear on any usages of the above.

It may be worth allowing the flag (or a separate option) to specify allowlisted files that are expected to have side-effects. Notably, webpack's sideEffects option is a boolean | string[] type for this purpose.

Comparison to bundler tree-shaking

This idea is inspired by bundler tree-shaking, but focuses on side-effects at compile-time rather than runtime. The ideas are similar but different enough to warrant a specific TypeScript flag.

For example, a file that calls console.log on import has a runtime side-effect bundlers may care about but does not have any affect on typing. The allowlisted array of files with side-effects will likely be different between TypeScript and bundlers.

Use Cases

Banning compile-time side-effects allows future speed improvements. Notably, the compiler can now know whether source files and its import tree can be safely elided from a Program if its exported identifiers are unused. This has potentially significant speed and memory improvements depending on the source file graph of a program.

A quick and unfinished implementation that only looks at export { ... } from "..." syntax was created in PR #40966.

Examples

For a program with the following files, TypeScript should output:

a.ts(2,0): 'someGlobalVar' is a global variable, which is banned by the 'noTypingSideEffects' compiler option.

// tsconfig.json
{
  "compilerOption": {
    "noTypingSideEffects": true
  },
  "files": ["index.ts", "a.ts"]
}

// index.ts
import { a } from "./a";

console.log(someGlobalVar);

// a.ts
export const a = 1;
declare var someGlobalVar = 2;

Checklist

My suggestion meets these guidelines:

[x] This wouldn't be a breaking change in existing TypeScript/JavaScript code
[x] This wouldn't change the runtime behavior of existing JavaScript code
[x] This could be implemented without emitting different JS based on the types of the expressions
[x] This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
[x] This feature would agree with the rest of TypeScript's Design Goals.

gluxon commented 3 years ago

I'd like to work on this if the TypeScript team is open to it and don't see any issues with the proposal. 🙂

RyanCavanaugh commented 3 years ago

This is a very noble goal but we'd need to see a lot more demand for it to incorporate it as a built-in flag since, as far as I can tell, there's nothing here that couldn't be detected with eslint

gluxon commented 3 years ago

Thanks for taking a look @RyanCavanaugh. I agree that the flag by itself would be better as a linter rule, but the intention was to open the door for compile-time unused source file elision. I believe that would require TypeScript itself to be aware of typing side-effects. I'm hoping we can merge a better form of PR #40966 in the future with this.

There's some numbers in the first PR that may be helpful copied here:

a large package on my team that got its createProgram time cut to 1/3 of the original time. (~30s to ~11s). Another large package we've had problems with went from ~28s to ~20s. I didn't give numbers initially because the differences depend heavily on project reference structure.

The limited feature suggestion here (a new flag) definitely depends on whether the TypeScript team wants to pursue unused source file elision though.

gluxon commented 3 years ago

@RyanCavanaugh Does the last comment change anything?

@sheetalkamat Wanted to tag you here as well since this is an area of TypeScript you've worked in and you've been helpful in the past.

sheetalkamat commented 3 years ago

we wouldnt want to do anything here until we have more feedback. Maintaining new flags that affect program structure is huge cost and needs to be well thought through with the set of users it benefits so as @RyanCavanaugh this could be definitely linter thing and if we have lot of people using it we could think about its usage in compiler

gluxon commented 3 years ago

That all makes sense. What are the steps to get more feedback? Do we want more examples of #40966 on some large open source projects perhaps?

RyanCavanaugh commented 3 years ago

Examples of good feedback include:

Novel problems that were encountered due to a lack of a feature
Lots of people who want the same thing, or are having a problem which this would solve
Evidence of need in the form of existing (but suboptimal) workarounds

evmar commented 3 years ago

I recently spent a long time in this area at Google and wrote up my results here, in particular the discussion around noResove towards the end.

In my experience on a massive codebase, the vast majority of code does not rely on type-level side effects, but then also random small places all over do end up using it. For one broad example, the lit-element ecosystem (which declares HTML custom components) wants to use declare global { interface HTMLTagNameMap { ... } } to register their new elements as arguments to document.createElement.

So I agree with the OP (in that changing the behavior here can have a big impact on build time), but also with the TS team that this is possibly too obscure to warrant a compiler flag.

I wonder if you could experiment with this at some level outside of TS. I am not so familiar with how multi-module builds work, but I could imagine something around gathering all the node_module entries that have opted into no type side effects and using that to control the list of inputs you pass to a TS compilation. That is more or less the solution we ended up with.

microsoft / TypeScript