Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

add typescript@rc #1051

Merged
merged 1 commit into from
Jun 13, 2017
Merged

add typescript@rc #1051

merged 1 commit into from
Jun 13, 2017

Conversation

aluanhaddad
Copy link
Collaborator

ESNext Dynamic Import!

ESNext Dynamic Import!
@aluanhaddad aluanhaddad force-pushed the aluanhaddad-patch-3 branch from 946b863 to db765e1 Compare June 13, 2017 11:09
@aluanhaddad aluanhaddad changed the title Create typescript@rc add typescript@rc Jun 13, 2017
@guybedford
Copy link
Member

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.

@guybedford guybedford merged commit 0d255cc into master Jun 13, 2017
@aluanhaddad
Copy link
Collaborator Author

@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 aluanhaddad deleted the aluanhaddad-patch-3 branch July 27, 2017 02:35
@aluanhaddad
Copy link
Collaborator Author

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

@guybedford
Copy link
Member

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 microsoft/TypeScript#16093 (comment).

@aluanhaddad
Copy link
Collaborator Author

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 microsoft/TypeScript#17444.

@aluanhaddad
Copy link
Collaborator Author

By the way, dynamic import is now available to TypeScript users in plugin-typescript as of frankwallis/plugin-typescript#209 released in 7.1.0

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

Successfully merging this pull request may close these issues.

2 participants