Doesn't handle dynamic imports

[amannn]

First of all, thanks a bunch for this package! I just realized today that apparently neither tsc nor @rollup/plugin-typescript are capable of outputting correct declaration files for an ESM package.

While trying ts-add-js-extension out on a project, I noticed one error though: Dynamic imports aren't rewritten correctly (reproduction).

Do you think that's in scope for this project?

EDIT: I think I'll just bite the bullet and use explicit file extensions everywhere 😔

[GervinFung]

@amannn sorry for replying after 1 week, been busy, and yes you are right, dynamic imports should be handled. Again apologise that u had to use explicit file extensions everywhere

[amannn]

Again apologise that u had to use explicit file extensions everywhere

Absolutely not your fault! Thanks anyway for building this tool, I was a bit surprised that this tooling gap in tsc actually exists …

Feel free to close this issue if you don’t intend to support this!

[GervinFung]

@amannn dynamic import case is handled, but is only limited to dynamic import with static string literal/template literal. Meaning - any dynamic import with path value that can only be obtained through runtime will not be handled, which is intended and AST alone can't resolve those kinds of stuffs

however, I do need help in making sure that I only alter dynamic import with string literal/template literal, if u can help me break the existing test b4 I publish it, that would be really helpful -> https://github.com/GervinFung/ts-add-js-extension/blob/fix/dynamic-imports/test/process/source/js/util/invalid-dynamic-import.js

[amannn]

if u can help me break the existing test b4 I publish it

My apologies, what would you like me to do? 😄 My original use case was static imports, so that should be covered then as far as I understand.

However, as mentioned above, I've anyway migrated to explicit file extensions now …

[GervinFung]

if u can help me break the existing test b4 I publish it

My apologies, what would you like me to do? 😄 My original use case was static imports, so that should be covered then as far as I understand.

However, as mentioned above, I've anyway migrated to explicit file extensions now …

I understand that u've manually added file extensions, I need help as in, are there any other ways to do "dynamic" value with static string? Without using variable/array

The 3 ways u've seen above are the only few ways I can come up with, if u can come up with more ways that i couldn't think of, that would be really helpful. Cos im trying to make sure i dont alter the wrong dynamic imports

[amannn]

I guess there can be some more:

// All kinds of string methods
import(str.substr(…));
import(['./module', 'c'].join(''))

// Completely dynamic
import(getPath());
import(path);

[GervinFung]

I guess there can be some more:

// All kinds of string methods
import(str.substr(…));
import(['./module', 'c'].join(''))

// Completely dynamic
import(getPath());
import(path);

Thanks for coming up with other variations, but variable/function/array can ve safely ignored. I only handle the case if the token after the ( of import is string literal or template literal. Array method is safe bcoz string.concat is ignored, so I think it's safe to publish. Thanks for raising this issue and offering help!