Skip to content

"Convert to ES6 module" not working with IIFE pattern #24297

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

Closed
mjbvz opened this issue May 21, 2018 · 6 comments
Closed

"Convert to ES6 module" not working with IIFE pattern #24297

mjbvz opened this issue May 21, 2018 · 6 comments
Labels
Fixed A PR has been merged for this issue

Comments

@mjbvz
Copy link
Contributor

mjbvz commented May 21, 2018

From @osya on May 21, 2018 8:59

  • VSCode Version: 1.23.1
  • OS Version: Windows 10 Pro x64

Steps to Reproduce:

  1. Create a file test_file.js with the following content:
var path = require('path');

(function () {
    var path = require('path');
}());

scr
Click lightbulb and select "Convert to ES6 module" then the 1st require transforms to the import path from "path";. It is correct. Then comment 1st require and click lightbulb and select "Convert to ES6 module" for the 2nd require. 2nd require doesn't converted to the import path from "path";. ES6 imports must be at the top level of your JavaScript files. So, I suppose, for require in IIFE there should be NO option "Convert to ES6 module"

Copied from original issue: microsoft/vscode#50205

@mjbvz
Copy link
Contributor Author

mjbvz commented May 21, 2018

Yes, I think not converting the inner require is correct since imports can only appear at the top level in a program.

In VS Code insiders, I do not see the convert to es6 suggestion on the inner require:

screen shot 2018-05-21 at 11 31 39 am

@osya
Copy link

osya commented May 21, 2018

@mjbvz , may be you just need to wait a little bit. There is a screenshot with existing "Convert to ES6 module" suggestion for require in the IIFE
scr

@DanielRosenwasser
Copy link
Member

(As an aside, could you tell us a bit more about why this IIFE is being used?)

@mhegazy
Copy link
Contributor

mhegazy commented May 21, 2018

@osya this last report should be fixed by #24101.

@mhegazy mhegazy added the Fixed A PR has been merged for this issue label May 21, 2018
@mhegazy mhegazy added this to the TypeScript 2.9.1 milestone May 21, 2018
@mhegazy mhegazy closed this as completed May 21, 2018
@osya
Copy link

osya commented May 21, 2018

@DanielRosenwasser I use IIFE for the following two reasons:

  1. I 'use strict';. Then without IIFE jshint generates warning: "Use the function form of "use strict". (W097)". So, the 1st reason for using IIFE is to avoid this warning.
  2. Resharper's warning "Property or global variable ... is possibly never assigned"

@ghost
Copy link

ghost commented May 21, 2018

@osya This was fixed recently in #24101, you may need to install typescript@next to stop seeing the suggestion in inner scopes.

@microsoft microsoft locked and limited conversation to collaborators Jul 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

4 participants