unimported icon indicating copy to clipboard operation
unimported copied to clipboard

Support pure ESM with TypeScript

Open chasingmaxwell opened this issue 3 years ago • 11 comments

I'm migrating a TypeScript project to output pure ESM. As part of that effort, I need to update every import which was previously extensionless, to specify a ".js" extension (even though the file in the source directory has a ".ts" extension). Like this:

// old way
import aThing from '../a/module';

// new way
import aThing from '../a/module.js';

It seems like unimported is unable to determine that ../a/module.js should be resolved to ../a/module.ts and so it reports almost all files as being unimported.

I've been referencing this guide for the migration to pure ESM.

chasingmaxwell avatar Dec 28 '21 14:12 chasingmaxwell

I think resolve always tries to resolve the import using the path + one of the allowed extensions. To support this case, we might need to check for the direct import first, before we pass the path to resolve.

Maybe we can add something right before the fallback:

https://github.com/smeijer/unimported/blob/b84910508b84fdf3233b18cb34390cd64ddac8cd/src/traverse.ts#L141-L145

// scratch / pseudo
try {
  const extension = config.extension.find(extension => path.endsWith(extension));
  if (extension) {
    return {
      type: 'source_file',
      path: resolve.sync(path.slice(0, -extension.length), {
        basedir: cwd,
        extensions: [extension],
        moduleDirectory: config.moduleDirectory,
      }).replace(/\\/g, '/'),
    }
  }
} catch (e) {}

smeijer avatar Dec 28 '21 17:12 smeijer

Looks like we'll have to add that in a few places though if we want it to work with aliases and module-relative paths as well.

chasingmaxwell avatar Dec 28 '21 18:12 chasingmaxwell

I could see it being applied to all relative paths across the board, but enabled with config. Either in a very targeted way with "esm": true or maybe more open-ended with regex path transformations or something "pathTransforms": { "/\.js/", ".ts" }.

chasingmaxwell avatar Dec 28 '21 18:12 chasingmaxwell

I'm curious if it would "just work" if we were to add an empty extension to the array. Like ['.js', '.tsx', '']

smeijer avatar Dec 28 '21 20:12 smeijer

When I tried that I received a "Failed parsing util" error message with exit code 1.

chasingmaxwell avatar Dec 28 '21 20:12 chasingmaxwell

I have it working for my project with an added pathTransforms config property. If that sounds good to you, I can create a pull request.

chasingmaxwell avatar Dec 28 '21 20:12 chasingmaxwell

A pull would be sweet! Thanks.

smeijer avatar Dec 29 '21 09:12 smeijer

I created the PR. A couple notes about tests:

  1. I could not get all the tests to pass locally (even on the main branch with no changes). It seems there might be a compatibility issue with my environment.
  2. I did not create any new tests. It was unclear to me where the test should be added. Maybe just a new scenario to the cli integration tests? If you can provide some guidance there I'll add a test for handling the new configuration property.

chasingmaxwell avatar Dec 29 '21 21:12 chasingmaxwell

Here's a PR to fix the build/test failures I was seeing on the main branch: https://github.com/smeijer/unimported/pull/69

chasingmaxwell avatar Jan 24 '22 19:01 chasingmaxwell

I've also added a test for the pathTransforms config in my PR #67

chasingmaxwell avatar Jan 24 '22 20:01 chasingmaxwell

@smeijer thank you for merging #69 (and sorry I never got back around to addressing your feedback). Now the checks for #67 are passing. If that's good to merge it'll close out this issue.

chasingmaxwell avatar May 02 '22 12:05 chasingmaxwell

:tada: This issue has been resolved in version 1.23.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

smeijer avatar Nov 15 '22 22:11 smeijer

Ran into an issue using this solution with .tsx, filed here: https://github.com/smeijer/unimported/issues/163

maclockard avatar Jun 16 '23 17:06 maclockard