Skip to content

FIXED: dynamic type imports #46

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

Merged
merged 6 commits into from
Dec 27, 2019
Merged

FIXED: dynamic type imports #46

merged 6 commits into from
Dec 27, 2019

Conversation

hedwiggggg
Copy link
Contributor

Resolve #45

Copy link
Member

@danielpza danielpza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I'll test it one more manually time before merging. Thanks a lot for your contribution 😄

@danielpza
Copy link
Member

@all-contributors add @hedwiggggg for bug, test, and code

@allcontributors
Copy link
Contributor

@danielpza

I've put up a pull request to add @hedwiggggg! 🎉

@hedwiggggg
Copy link
Contributor Author

hedwiggggg commented Dec 26, 2019

I've tested it myself a little bit;
ImportTypeNode is type-only

so this for example won't work:

async function main(): Promise<void> {
  const Logger = (await import('@/test')).Logger;
  console.log(new Logger());
}

main();

@hedwiggggg
Copy link
Contributor Author

Okay, I think I could find a pattern to get the corresponding nodes for async imports.

    if (ts.isCallExpression(node)) {
      if (node.expression.kind === ts.SyntaxKind.ImportKeyword) {
        console.log(node);
      }
    }

Idk, if it's possible to simplify this; Unfortunately it seems to be not as easy as the type-only imports.

@hedwiggggg
Copy link
Contributor Author

It turned out that the pattern is not so different from the require statement detection. I extended the tests and refactored the code a bit.

Copy link
Member

@danielpza danielpza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you for your contribution :-)

@danielpza danielpza merged commit 88b6001 into LeDDGroup:master Dec 27, 2019
@danielpza
Copy link
Member

included in v1.1.14

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

Successfully merging this pull request may close these issues.

Seems to be not working with dynamic import expression
2 participants