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

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

Closed
kryops opened this issue Aug 2, 2017 · 6 comments
Closed

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

kryops opened this issue Aug 2, 2017 · 6 comments
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue Good First Issue Well scoped, documented and has the green light Help Wanted You can do this

Comments

@kryops
Copy link

kryops commented Aug 2, 2017

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:

  • "module": "commonjs" emits var isUrl = require("is-url");
  • "module": "es2015" errors with index.ts(1,1): error TS1202: Import assignment cannot be used when targeting ECMAScript 2015 modules. Consider using 'im port * as ns from "mod"', 'import {a} from "mod"', 'import d from "mod"', or another module format instead.

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 DanielRosenwasser added Help Wanted You can do this Bug A bug in TypeScript labels Aug 2, 2017
@DanielRosenwasser DanielRosenwasser added the Good First Issue Well scoped, documented and has the green light label Aug 2, 2017
@DanielRosenwasser
Copy link
Member

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!

@DanielRosenwasser DanielRosenwasser added this to the TypeScript 2.5 milestone Aug 2, 2017
@ruimprego
Copy link

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

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Aug 3, 2017

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
Copy link
Member

DanielRosenwasser commented Aug 3, 2017

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
Copy link
Contributor

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
Copy link

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.

@mhegazy mhegazy assigned weswigham and unassigned rbuckton Aug 31, 2017
@weswigham weswigham added this to Not started in Rolling Work Tracking Sep 5, 2017
@weswigham weswigham moved this from Not started to In Review in Rolling Work Tracking Sep 7, 2017
@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Sep 11, 2017
@weswigham weswigham removed this from In Review in Rolling Work Tracking Sep 13, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
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 Good First Issue Well scoped, documented and has the green light Help Wanted You can do this
Projects
None yet
Development

No branches or pull requests

8 participants