microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.29k stars 12.39k forks source link

Expose ts.matchFiles as public API to make implementing readDirectory easier. #13793

Open krisselden opened 7 years ago

krisselden commented 7 years ago

Problem:

When implementing a custom host that virtualizes the file system, the readDirectory method is cumbersome to implement in a way that matches tsc behavior.

If you look at the hosts within typescript, the matchFiles method was added to unify behavior with readDirectory between hosts.

While matchFiles is exported, it isn't in the type definition files so I'm assuming it is "private" API.

Either the host should be able to provide a getFileSystemEntries(path: string) => { files: string[], directories: string[] } to get a readDirectory implementation or expose matchFiles.

I'm working on improving the broccoli-typescript-compiler broccoli plugin to have more parity with tsc and VS Code, but I need to virtualize the FS, since broccoli builds from tmp directory and I want the node_modules/@types discovery, tsconfig extends, etc all to work, I need to pretend the tmp dir is inside the project.

Thanks

mhegazy commented 7 years ago

Our policy has been to only expose APIs that represent language constructs/concepts; helpers and utilities are kept private to limit the API surface area.

This specific API falls under the utilities category, and it kinda falls between two abstraction levels (System, and CompilerHost). We have generally responded to similar requests by asking about duplicating the code, specially for small helpers, would that be an option?

If understand correctly, you are implementing System, and not not using the existing ts.sys, if so, can you use ts.sys?

krisselden commented 7 years ago

@mhegazy you cannot make a host that reasonably supports globbing that virtualizes the file system, look at your own examples of hosts, ts.sys doesn't work here because it doesn't allow the file system to be virtualized.

The globing is somewhat unique to TypeScript, it doesn't quite align with other node glob libraries so if you want to ensure your host behaves similar to tsc would require using private API or copying and pasting a large amount of code.

I need to virtualize the file system to fit into how broccoli build system works in tmp dirs, but give errors and resolve config and node_modules from the project dir.

krisselden commented 7 years ago

@mhegazy I generally understand the policy regarding API surface but there is a massive amount of useful things in this codebase that is private API, stuff that would make for some amazing static analysis like the flow, container, bindings, imports, exports. Also, non of the code generation stuff is exposed, no plugins etc. Exposing this stuff could make for some wickedly cool satellite projects. I understand not wanting to back yourself into a corner with the API but sometimes worse is better.

mhegazy commented 7 years ago

The code is open source, and has always been, so to be clear we do not intend to hide anything.

The main issue is that more API surface area means more promises to not break API shape and semantics, and we really hate to break ppl. What complicates matters is we have certain use cases, for APIs, these are the things we test for, other use cases could be broken with no notification.

For the emitter, we have been working on exposing these, see https://github.com/Microsoft/TypeScript/issues/13765, https://github.com/Microsoft/TypeScript/issues/13764 and https://github.com/Microsoft/TypeScript/issues/13763. The printer API is already in, see https://github.com/Microsoft/TypeScript/pull/13761, and the factory is in review, see https://github.com/Microsoft/TypeScript/pull/13825.

For other APIs, please let us know what APIs you want to use, and how you intend to use them, and we would be happy to expose them. we just need to make sure we understand how users are using the API to make sure we know what changes we can and what we can not make in the future.

As for the original issue.. if you really, really, really need it, I suppose we can expose it.

dsherret commented 7 years ago

+1, I am working on trying to find all the source files associated with a tsconfig.json and using the existing sys.readDirectory isn't an option because it doesn't work in a virtualized file system.

It would be nice to reuse the code in ts.matchFiles.

krisselden commented 7 years ago

For now I am just relying on the fact it is exported and hoping it won't break but I consider readDirectory to be a poor contract for a host to be able to provide and hope to ensure the include/exclude function the way people expect.

AviVahl commented 5 years ago

@mhegazy can this be looked at again? I've encountered the exact same need.

I have my own implementation for an in-memory file system which I would like to use for tests and caching. Implementing a ParseConfigHost or LanguageServiceHost is easy up to the point where you have to provide a custom readDirectory. I order to get the exact same behavior of TypeScript, I shamelessly augmented the types:

import * as ts from 'typescript'
declare module 'typescript' {
    function matchFiles(
        path: string, extensions: ReadonlyArray<string>, excludes: ReadonlyArray<string>,
        includes: ReadonlyArray<string>, useCaseSensitiveFileNames: boolean, currentDirectory: string,
        depth: number | undefined, getFileSystemEntries: (path: string) => FileSystemEntries): string[]

    export interface FileSystemEntries {
        files: ReadonlyArray<string>
        directories: ReadonlyArray<string>
    }

    export interface JsonSourceFile extends ts.SourceFile {
        parseDiagnostics: ts.Diagnostic[]
    }
    export function getNewLineCharacter(options: ts.CompilerOptions | ts.PrinterOptions): string
}

This can obviously break at any given future TS release without me knowing at build time.

Also, even with matchFiles() exposed, I had to implement:

    function getFileSystemEntries(path: string): ts.FileSystemEntries {
        const files: string[] = []
        const directories: string[] = []
        const entries = syncFs.listDirectorySync(path)
        for (const entry of entries) {
            const node = syncFs.getStatsSync(syncFs.path.join(path, entry))
            if (node.isFile()) {
                files.push(entry)
            } else if (node.isDirectory()) {
                directories.push(entry)
            }
        }
        return {files, directories}
    }

The parseDiagnostics field and getNewLineCharacter() helper are two additional symbols I found useful when creating a custom language service.

cspotcode commented 5 years ago

@AviVahl I'm trying to do the same thing and just encountered readDirectory myself. My goal is to publish an in-memory implementation of ts.System for use in browsers demos, experiments, etc. Do you have a link to what you're working on?

AviVahl commented 5 years ago

@cspotcode https://github.com/AviVahl/ts-tools/blob/master/packages/typescript-service/src/create-host.ts

usergenic commented 4 years ago

I'm currently having to implement a readDirectory etc and I was really hoping this had been addressed 3 years ago when brought up.