A require call that isn't the global require will clobber bindings

[krisselden]

TypeScript Version: 1.8.0 / nightly (2.0.0-dev.201xxxxx)
typescript@beta or typescript@next
Code

// A *self-contained* demonstration of the problem follows...
import require from "some/module"

export foo from "some/other/module"

require("a single arg")

Expected behavior:
has a binding for foo
Actual behavior:
require() call expression is not checked for scope, that it is the module's require or that externalModuleIndicator already is present, it will redefine the symbol.

workaround in a custom host getSourceFile

      Object.defineProperty(sourceFile, "commonJsModuleIndicator", {
        get: () => sourceFile["externalModuleIndicator"]
      });

[mhegazy]

export foo from "some/other/module"

you mean export {foo} from "some/other/module"

require() call expression is not checked for scope,

I do not understand what you mean. can you provide more context.

[krisselden]

sorry, here is the real example

https://github.com/emberjs/ember.js/blob/master/packages/ember-metal/lib/index.js#L181

[mhegazy]

and what is wrong with this require call? i did not understand the original problem.

[krisselden]

It hits here, it already has the symbol, an externalModuleIndicator but it is going to assign file.symbol = again
https://github.com/Microsoft/TypeScript/blob/master/src/compiler/binder.ts#L1870

[krisselden]

This require is not node's or commonjs require, it is local, but the call expression does not check where the function came from.

[krisselden]

I suppose I can rename the import to something else so it doesn't confuse the binder but seems annoying since it actually knows this require is local but it does a very simple check https://github.com/Microsoft/TypeScript/blob/master/src/compiler/utilities.ts#L1296

[mhegazy]

is this a .js file?

[krisselden]

Yes

[krisselden]

We have some typescript in the project, right now we transpile it separately, I'm working on compiling it together so it is easier to add more or convert.

[krisselden]

I guess I could do import eriuqer from "require"; to get the require from our custom loader or pass a dummy second argument but it seems like it should at least not clobber the existing symbol or that it is already recognized it as a ES2015 module.

[mhegazy]

thanks, i get it now.

[mhegazy]

no wait.. i misspoke.. here is my repro:

C:\test\sandbox7>tsc --v
Version 2.1.0-dev.20160912

C:\test\sandbox7>type src\m.js
import require, { has } from "./c"

if (has('ember-debug')) {
  require('ember-debug');
}
C:\test\sandbox7>type src\c.js
export default function require(a) { }

export function has(a) { return true }

C:\test\sandbox7>tsc --allowjs src\m.js --outDir dist

C:\test\sandbox7>type dist\m.js
"use strict";
var c_1 = require("./c");
if (c_1.has('ember-debug')) {
    c_1["default"]('ember-debug');
}

C:\test\sandbox7>type dist\c.js
"use strict";
function require(a) { }
exports.__esModule = true;
exports["default"] = require;
function has(a) { return true; }
exports.has = has;

[mhegazy]

what did i miss?