jspm / registry

The jspm registry and package.json override service
https://jspm.io
229 stars 256 forks source link

add typescript@rc #1051

Closed aluanhaddad closed 7 years ago

aluanhaddad commented 7 years ago

ESNext Dynamic Import!

guybedford commented 7 years ago

Awesome, I haven't had a moment to look into the implementation - will be interesting to see if they are emitting the System.register support for it.

aluanhaddad commented 6 years ago

@guybedford I got a chance to look into this a bit recently and the results seem promising. Compiling the following code

import('jquery').then(({default: $}) => {
  $('div').text('您好');
});

into the System.register module format using TypeScript results in the following JavaScript

System.register([], function (exports_1, context_1) {
    var __moduleName = context_1 && context_1.id;
    return {
        setters: [],
        execute: function () {
            context_1.import('jquery').then(({ default: $ }) => {
                $('div').text('您好');
            });
        }
    };
});

Does this look correct?

If I understand the intent of the spec correctly, it is possible for the behavior of a dynamic import to be affected by arbitrary dynamic context associated with the importing module which is why the import is transpiled into context_1.import as opposed to SystemJS.import, but I could be way off on that.

There is also a new module target esnext which results in retention of dynamic imports along with other ESM constructs which seems ideal for scenarios that pipe the results through plugin-babel or take advantage of Rollup in the SystemJS Builder (when dynamic import support is added).

Unfortunately, there appears to be bug on the type checking side that fails to apply the --allowSyntheticDefaultImports logic to dynamically loaded modules and therefore does not acknowledge that 'jquery' will have a default property. This doesn't affect emit.

aluanhaddad commented 6 years ago

I just re-read your comment in the TS repo and this does indeed appear to be correct.

guybedford commented 6 years ago

Thanks for checking this out, great to hear it's working ok. Yes exactly the context allows relative module names to work out relative to the runtime parent.

Unfortunately, there appears to be bug on the type checking side that fails to apply the --allowSyntheticDefaultImports logic to dynamically loaded modules and therefore does not acknowledge that 'jquery' will have a default property. This doesn't affect emit.

Oh dear, well hopefully this can be sorted out soon. I still wish TypeScript would just get on board with https://github.com/Microsoft/TypeScript/issues/16093#issuecomment-304263496.

aluanhaddad commented 6 years ago

I absolutely agree. The good news is that the TypeScript team does seem to be recommending the import = syntax more and more frequently.

Unfortunately, there is a seemingly endless plethora of tutorials and even official documentation for libraries such as moment.js that give bad advice to TypeScript users who continue to write more and more non standard code that will turn into broken code.

With respect to the async import type checking bug, I filed https://github.com/Microsoft/TypeScript/issues/17444.

aluanhaddad commented 6 years ago

By the way, dynamic import is now available to TypeScript users in plugin-typescript as of https://github.com/frankwallis/plugin-typescript/pull/209 released in 7.1.0