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

Explore supporting ES import() proposal #12364

Closed
mohsen1 opened this issue Nov 18, 2016 · 34 comments
Closed

Explore supporting ES import() proposal #12364

mohsen1 opened this issue Nov 18, 2016 · 34 comments

Comments

@mohsen1
Copy link
Contributor

@mohsen1 mohsen1 commented Nov 18, 2016

Dynamic import proposal is in early stages but it will require TypeScript to support it in order to be usable in TypeScript based apps.

Basic syntax:

import('./my-module.ts').then(({export1, export2}) => {
  
});
@mhegazy
Copy link

@mhegazy mhegazy commented Nov 18, 2016

This is a proposal that we are following in the committee. I would expect us to start looking into this early next year.

also referenced in #11611. i think we just need one issue to track this.

@Jessidhia
Copy link

@Jessidhia Jessidhia commented Nov 22, 2016

Stubbing it such that it is not a syntax error, has apparent type (request: string) => Promise<any>, and is emitted as-is could be useful for using it with e.g. https://github.com/airbnb/babel-plugin-dynamic-import-webpack or eventually with webpack/webpack#3098 in webpack 2; but I don't know how good or bad an idea that is.

@aluanhaddad
Copy link
Contributor

@aluanhaddad aluanhaddad commented Nov 22, 2016

@Kovensky anyone can easily write that stub should they need it so I don't think its declaration belongs in the language any more than require, define, or System.import.

@Jessidhia
Copy link

@Jessidhia Jessidhia commented Nov 22, 2016

@aluanhaddad not quite. Just writing a ( after import causes a syntax error, regardless of anything you try to do declaration-wise. This needs compiler support, and definitely behind an experimental flag if it's ever done as a stub.

@aluanhaddad
Copy link
Contributor

@aluanhaddad aluanhaddad commented Nov 27, 2016

@Kovensky I don't think I have actually tried to write it like that, thanks for explaining that this isn't possible.

@Jessidhia
Copy link

@Jessidhia Jessidhia commented Dec 4, 2016

Dynamic import() support landed in webpack master and should be in the next webpack 2 beta. It was also promoted to stage-3 as of the last latest TC39 meeting.

@saschanaz
Copy link
Contributor

@saschanaz saschanaz commented Dec 4, 2016

It was also promoted to stage-3 as of the last TC39 meeting

That means SystemJS may update their API to match it! :D

@mohsen1 mohsen1 changed the title Explore supporting dynamic import Explore supporting ES import() proposal Dec 4, 2016
@chicoxyzzy
Copy link
Contributor

@chicoxyzzy chicoxyzzy commented Dec 7, 2016

SystemJS can't update their API to match proposal because import() is a SyntaxError. TypeScript's parser should definitely support this syntax so people will be able to transpile dynamic imports to SystemJS / CommonJS / Webpack 1 require.ensure() / use WebPack 2 code splitting

@chicoxyzzy
Copy link
Contributor

@chicoxyzzy chicoxyzzy commented Dec 7, 2016

has apparent type (request: string) => Promise<any>

it's (specifier: any) => Promise<any>, according to latest spec, it's totally valid to pass anything to it, ToString will be applied to specifier

@chicoxyzzy
Copy link
Contributor

@chicoxyzzy chicoxyzzy commented Dec 7, 2016

Can we have a compiler option to support this syntax?

@chicoxyzzy
Copy link
Contributor

@chicoxyzzy chicoxyzzy commented Dec 7, 2016

Sadly we'll loose type information when specifier should be computed :(

@Jessidhia
Copy link

@Jessidhia Jessidhia commented Dec 7, 2016

There are talks about attaching meta-properties to import, similarly to what was done with new for new.target. It might be better represented (for forward compatibility) as an interface with that call signature, but this might be thinking too far ahead.

See estree/estree#137 (comment)

@bmeck
Copy link

@bmeck bmeck commented Dec 7, 2016

@Jessidhia
Copy link

@Jessidhia Jessidhia commented Jan 6, 2017

@mhegazy is this going to be in the TS "Future" (2.3?) roadmap?

@originalmoose
Copy link

@originalmoose originalmoose commented Jan 6, 2017

I have a solution if anyone wants a hack to use while waiting for the feature to be properly implemented in TS.

You'll need string-replace-loader or something similar setup as the last loader in your ts config. (The search and replace should probably be a regex just to make sure we don't accidentally clobber something but this was enough to test that the idea works)

module: {
    rules: [
        {
            test: /\.tsx?$/,
            use: [
                {
                    loader: 'string-replace-loader',
                    options: {
                        search: '_import(',
                        replace: 'import('
                    }
                },
                {
                    loader: 'babel-loader'
                },
                {
                    loader: 'ts-loader',
                    options: {
                        silent: true,
                        compilerOptions: {
                            noEmit: false,
                        },
                    },
                },
            ],
        },
    ]
}

Then simply create a definition like so...

declare function _import<T>(path: string): Promise<T>;

You can change the _import to really be anything but I felt the _ was pretty safe and would be easy enough to find and replace when typescript adds support.

My test was a simple react-router v4 app with two routes that were added with _import. An example of it in use is below

render() {
    return (
        <Match pattern="/" exactly render={props =>
            <LazyRoute module={() => _import('./Home')}>
                {module => module ? <module.default {...props} /> : null}
            </LazyRoute>
        } />
    );
}
shlomiassaf pushed a commit to shlomiassaf/ng-router-loader that referenced this issue Jan 17, 2017
Shlomi Assaf (shlassaf)
Support the [Dynamic import proposal](https://github.com/tc39/proposal-dynamic-import) [TC39 Stage 3] as a loader (codegen).
The import() construct is supported in webpack from 2.1.0-beta28

Also adds decprecation message then using async-system (System.import) as it will be removed from webpack 3+

BREAKING CHANGES:
Typescript transform `import(...)` syntax into something.
To use `ng-router-loader` with `async-import` code generator the `ng-router-loader` must
 run **AFTER** the TS compiler (e.g: awesome-typescript-loader), that is in a lower index in the loaders array.
 This also requires the code generators to emit ES5 code.
SEE: microsoft/TypeScript#12364
@Avol-V
Copy link

@Avol-V Avol-V commented Jan 19, 2017

It's hard to use such hack without native support if we want types. We should do things like:

import * as ModuleInit from './init';

async function main(): Promise<void>
{
	const Init: typeof ModuleInit = await _import( './init' );
	
	Init.default();
}
@isiahmeadows
Copy link

@isiahmeadows isiahmeadows commented Feb 14, 2017

I'll note that the proposal is currently at stage 3, and only string literals should be checked (anything else should return {[key: string]: any}).

Also, it'd be nice to see it transpiled in CommonJS/AMD (Webpack 2 works with ES module syntax):

// Original
import("specifier").then(...)

// CommonJS
Promise.resolve()
.then(() => require("specifier"))
.then(...)

// AMD
Promise.resolve()
.then(() => new Promise(resolve => require(["specifier"], resolve)))
.then(...)

In particular, lazy loading in Node would be very nice for end-user apps with optional features, and import() provides a very nice way of doing that.

@mohsen1
Copy link
Contributor Author

@mohsen1 mohsen1 commented Feb 14, 2017

Promise will resolve to module namespace not any.

import * as path from 'path';
console.log(path.resolve('foo'));

and

import('path').then(path => {
  console.log(path.resolve('foo'));
});

I'm assuming TypeScript will not allow dynamic module specifier (import(moduleName + '.ts')) as spec is suggesting.

@Jessidhia
Copy link

@Jessidhia Jessidhia commented Mar 10, 2017

It looks like the result will be typed as Promise<any>. Maybe in the future improvements in the way of #13231 could help better type the return type; but you'd probably still need to do it with asserts.

@AlanWuji
Copy link

@AlanWuji AlanWuji commented Mar 21, 2017

Is there any plan to support import() now ?

@maierson
Copy link

@maierson maierson commented Mar 21, 2017

I'm also interested in this for code splitting. This doesn't work at the moment because of import() call:

import Loadable from 'react-loadable';

function MyLoadingComponent() {
  return <div>Loading...</div>;
}

const LoadableAnotherComponent = Loadable({
  loader: () => import('./another-component'),
  LoadingComponent: MyLoadingComponent
});

class MyComponent extends React.Component {
  render() {
    return <LoadableAnotherComponent/>;
  }
}
@Jessidhia
Copy link

@Jessidhia Jessidhia commented Mar 21, 2017

For now, you can work around it by having

declare global {
  interface System {
    import (request: string): Promise<any>
  }
  var System: System
}

and using System.import instead of import. webpack 2 provides the same behavior under that alias, though it will probably get removed in webpack 3. You will, unfortunately, not get type checking, though, but there are ways to get something good enough for now working.

@maierson
Copy link

@maierson maierson commented Mar 21, 2017

@Kovensky that worked great. Thank you.

A few gotchas - webpack 2 needs a chunkFilename to load the chunk script correctly:

  output: {
    filename: '[name].js',
    path: PATHS.build,
    publicPath: '/',
    chunkFilename: '[id].chunk.js'
  },

Also used the babel-plugin-syntax-dynamic-import in .babelrc

If you are getting webpackJsonp errors make sure that the scripts are in the correct order in .html (vendor, app then chunks)

@maierson
Copy link

@maierson maierson commented Mar 21, 2017

Also for react-loadable it only works with default exports #20

@jch254
Copy link

@jch254 jch254 commented May 8, 2017

@Kovensky cheers for the workaround! I've combined with react-loadable here if anyone is interested in a demo: https://github.com/jch254/starter-pack/tree/typescript.

@napalm684
Copy link

@napalm684 napalm684 commented May 10, 2017

@Kovensky while this post is largely around react, I am attempting your interface approach via Angular 2 and still getting the import property is undefined? Any thoughts:

main.ts:20 Uncaught TypeError: Cannot read property 'import' of undefined
    at getTranslations (main.ts:20)
    at Object.<anonymous> (main.ts:12)
    at __webpack_require__ (bootstrap 8581ef0…:52)
    at Object.<anonymous> (module.js:22)
    at __webpack_require__ (bootstrap 8581ef0…:52)
    at webpackJsonpCallback (bootstrap 8581ef0…:23)
    at app.8581ef0….js:1

Generated code looks as follows I am wondering if the interface is not being ignored in this case causing the actual import() function to go un-recognized.

var system_interface_1 = __webpack_require__(508);
core_1.enableProdMode();
// platformBrowserDynamic().bootstrapModule(AppModule);
getTranslations();
function getTranslations() {
    var locale = document['locale'] ? document['locale'] : 'en';
    switch (locale) {
        case 'en':
            system_interface_1.System.import('../locale/messages.en.xlf!text').then(function (someModule) {
                platform_browser_1.platformBrowser().bootstrapModuleFactory(app_module_ngfactory_1.AppModuleNgFactory);
            });
            break;
    }
}
/***/ }),
/* 508 */
/***/ (function(module, exports, __webpack_require__) {

"use strict";
// TODO: Remove when TS supports dynamic imports: https://github.com/Microsoft/TypeScript/issues/12364

Object.defineProperty(exports, "__esModule", { value: true });
// tslint:enable 
//# sourceMappingURL=system.interface.js.map

/***/ }),
@mhegazy
Copy link

@mhegazy mhegazy commented Jun 6, 2017

Fixed by #14774

@mhegazy mhegazy closed this Jun 6, 2017
@mhegazy mhegazy added this to the TypeScript 2.4 milestone Jun 6, 2017
@Jessidhia
Copy link

@Jessidhia Jessidhia commented Jun 6, 2017

It was reverted 😢

#16281 should actually fix it.

EDIT: now it's merged 🎉

@misantronic
Copy link

@misantronic misantronic commented Jul 2, 2017

I am still having this issue - using ts@2.4.1

@samuelmaddock
Copy link

@samuelmaddock samuelmaddock commented Oct 4, 2017

@misantronic You might need to change your tsconfig's "module" option to "esnext"

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.