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

Incorrect assumption around host being defined in base64encode/base64decode #24638

Closed
kitsonk opened this issue Jun 3, 2018 · 2 comments
Closed
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@kitsonk
Copy link
Contributor

kitsonk commented Jun 3, 2018

TypeScript Version: 2.9.1+

Code

Any code under deno with TypeScript 2.9.1 or later.

#22658 introduced a significant refactor of code. One of the areas defines the base64encode function which assumes that the argument host is always defined. In practice, this comes from ts.sys in which ts.sys is possibly undefined, but without the strict checking, this does not throw a compiler error, but at runtime in certain environments (like deno) an exception is thrown.

A simple check of host && host.base64encode there (and the following function base64decode restores functionality).

Expected behavior:

The code actually runs.

Actual behavior:

TypeError: Cannot read property 'base64encode' of undefined
    at Object.base64encode (../node_modules/typescript/lib/typescript.js:12079:18)
    at Object.getSourceMappingURL (../node_modules/typescript/lib/typescript.js:73929:46)
    at printSourceFileOrBundle (../node_modules/typescript/lib/typescript.js:74550:48)
    at emitJsFileOrBundle (../node_modules/typescript/lib/typescript.js:74499:13)
    at emitSourceFileOrBundle (../node_modules/typescript/lib/typescript.js:74456:13)
    at forEachEmittedFile (../node_modules/typescript/lib/typescript.js:74365:30)
    at Object.emitFiles (../node_modules/typescript/lib/typescript.js:74446:9)
    at emitWorker (../node_modules/typescript/lib/typescript.js:78992:33)
    at ../node_modules/typescript/lib/typescript.js:78952:66
    at runWithCancellationToken (../node_modules/typescript/lib/typescript.js:79043:24)

Related Issues:
Refs: ry/deno#117

@kitsonk
Copy link
Contributor Author

kitsonk commented Jun 3, 2018

Actually, in looking a bit deeper, .sys is defined as always available, but the code internal to creating it logically assumes it can be undefined and there are quite a few uses of .sys it is assumed to be there. There is this TODO which indicates this has been noticed before.

I tried changing it to be | undefined and the whole sweater came unravelled so it clearly needs a bit more guidance to long term fix it.

@mhegazy mhegazy added the Bug A bug in TypeScript label Jun 4, 2018
@mhegazy mhegazy added this to the TypeScript 3.0 milestone Jun 4, 2018
@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Jun 4, 2018
@mhegazy
Copy link
Contributor

mhegazy commented Jun 4, 2018

thanks @kitsonk

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

2 participants