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

import = require is silently swallowed with module: esnext #17556

Closed kryops closed 7 years ago

kryops commented 7 years ago

TypeScript Version: 2.4.2 / nightly (2.5.0-dev.20170801)

Code

import isUrl = require('is-url')
console.log(isUrl('http://www.foo.bar'))

tsconfig.json

{
    "compilerOptions": {
        "target": "es5",
        "module": "esnext",
        "strict": true
    }
}

Expected behavior: Not entirely sure what is expected to happen here:

Actual behavior: The compilation finishes without an error, but the import is missing in the output:

"use strict";
console.log(isUrl('http://www.foo.bar'));
DanielRosenwasser commented 7 years ago

Thanks for the report! This should just be a matter of issue of

  1. Adding a test for this in tests/cases/compiler

    // @module: esnext
    // @target: es5
    import hi = require('hello');
    hi();
  2. Rewording the error message (just drop the 2015) part
  3. Updating diagnostics and references to that diagnostic (jake generate-diagnostics)
  4. Issuing the error on ES2015 or ESNext module targets.

We'd be willing to take a PR if you're interested!

ruimprego commented 7 years ago

I wouldn't mind tackling this, if needed, but I would need some guidance, since this would be my first PR.

DanielRosenwasser commented 7 years ago

Go for it! Some folks have recently been out and are catching up on bugs, but we'll try our best to give feedback. 🙂

DanielRosenwasser commented 7 years ago

Check out our Instructions for Contributing Code to TypeScript on our CONTRIBUTING.md. I think if you don't submit a CLA, a bot will prompt you when you send your PR.

You can also look at some of my recent PRs (e.g. #17459) for inspiration. The workflow I use is

  1. Add failing test and confirm it fails in some observable way.
  2. Apply new logic.
  3. Run tests to confirm that baselines have appropriately changed, or that the test now passes.
john-patterson commented 7 years ago

I started looking at this. This probably also needs a change for export assignment as well. I tried

import foo = require("foo");
foo();
export = foo;

With (module = esnext, target = es5) I get this error app.ts(1,22): error TS2307: Cannot find module 'foo'. and the following code is emitted:

foo();

With (module = es2015, target = es5), the following errors are emitted:

app.ts(1,1): error TS1202: Import assignment cannot be used when targeting ECMAScript 2015 modules. Consider using 'import * as ns from "mod"', 'import {a} from "mod"', 'import d from "mod"', or another module format instead.
app.ts(1,22): error TS2307: Cannot find module 'foo'.
app.ts(3,1): error TS1203: Export assignment cannot be used when targeting ECMAScript 2015 modules. Consider using 'export default' or another module format instead.

and the following code is emitted:

foo();

I'll continue working on this and hopefully have a PR sometime this weekend unless you've make progress @captainSpades

fictitious commented 7 years ago

It looks like ES modules are eventually going to be supported natively in node.js. The intended purpose of import assignment is interoperability with node.js, so wouldn't it make sense to support that for module=esnext ?

The way things are headed now, there will be severe limitations on interoperability between ES modules and commonjs modules in node. If I write a library targeting node, for the time being I would likely need to distribute two sets of compiled files - one for commonjs consumers, another one to be used in new node applicatinons which are targeting ES modules. If my library has commonjs dependency, it's OK, but that dependency needs to be used with var x = require("x") in generated commonjs code, and with default import import x from "x" in generated ES module code (at least that's what people who seem to know where node.js is going are saying).

The problem is, I can't find a combination of options that will allow me to do that from single set of source files and type definitions. Transpiling import x = require("x") to import x from "x" when module=esnext would solve it nicely I think.