Js-Brecht / babel-plugin-tsconfig-paths

Resolve imports/requires based on path aliases in tsconfig.json
ISC License
10 stars 3 forks source link

Original extension always being added - not working with typescript transpile #1

Open IlyaSemenov opened 4 years ago

IlyaSemenov commented 4 years ago

Steps to reproduce

Create src/foo.ts:

import baz from "~/bar/baz"

create src/bar/baz.ts:

console.log("I am baz")

create tsconfig.json:

{
    "compilerOptions": {
        "baseUrl": ".",
        "paths": {
            "~/*": [
                "src/*"
            ],
        },
    }
}

create .babelrc:

{
    "presets": [
        [
            "@babel/env",
            {
                "targets": {
                    "node": true
                }
            }
        ]
    ],
    "plugins": [
        "@babel/transform-typescript",
        "tsconfig-paths"
    ]
}

and compile the project with:

babel src --extensions '.js,.ts' --out-dir build 

then start it with:

node build/foo

Expected result

The script runs normally.

Actual result

The script crashes:

Error: Cannot find module './bar/baz.ts'

If you check build/foo.js, you will see:

var _baz = _interopRequireDefault(require("./bar/baz.ts"));

You see the problem? The plugin keeps the original source file extension, which is replaced on babel compile.

The problem is in this code: https://github.com/Js-Brecht/babel-plugin-tsconfig-paths/blob/a534e4ddb9632eebd73af9c429991cab031d9465/lib/resolve.js#L83-L91

If I change it like this:

-       ? getRelativePath(sourceFile, realFile) 
+       ? getRelativePath(sourceFile, checkImport) 
-       : realFile 
+       : checkImport 

that fixes the problem.

fmal commented 2 years ago

@Js-Brecht this plugin would fit very well into my workflow except that i'm also running into this issue, any chance this could get fixed? Thank you :)

Js-Brecht commented 2 years ago

Hey guys, I’m really sorry, not sure how this ticket fell through the cracks.

I do see the problem. It certainly does seem like using the transformed import less the file extension should be sufficient.

There may be other things to consider, however. This is generally pretty effective when using Webpack and other bundlers because it is actually targeting the existing source file, instead of a build artifact. Using checkImport shouldn’t hurt anything in those cases either, but I will want to do some testing to make sure.

Thanks for bringing this to my attention

ondrej-111 commented 2 years ago

Currently, I am using 1.0.3-rc.1 version which removes file extensions from imports - so works correctly nicely for me :)

_interopRequireDefault(require("../../common/config/config.wrapper"))
tconroy commented 2 years ago

is this resolved with 1.0.3? Or do you need to use the rc.1? I'm using 1.0.3 but still encountering this issue.

EDIT: Yup, looks like needs rc.1. This fixed it for me.

Js-Brecht commented 2 years ago

Sorry for the confusion. It’s rc.1… published the fix for testing and ran out of time for deploying the official version. I may be able to make time to wrap this up tonight, but otherwise it will probably be this weekend.

listanegra commented 2 years ago

I had this same problem, but noticed something different happening when using ES6 Modules. I think instead of removing completely the file extension, we should keep it and convert to the expected output extension, because when using ES6 Modules on Node.JS prior to v14, the file extension .js is a requirement when importing modules.

In the example provided by @IlyaSemenov , the generated output would be the following: var _baz = _interopRequireDefault(require("./bar/baz.js"));

Assuming the original code was transpiled into ES6 Modules syntax, the generated output would be the following instead: import baz from "./bar/baz.js"

I attached the patch diff generated by the code I made and tested locally, I think this is the ideal fix instead of removing the file extension completely, since it maintains compatibility with ES6 Modules and also works for CommonJS requires as well

diff --git a/lib/resolve.js b/lib/resolve.js
index e751e34..65b1e0f 100644
--- a/lib/resolve.js
+++ b/lib/resolve.js
@@ -17,6 +17,25 @@ const getRelativePath = (from, to) => {
     : `./${relPath}`
 }

+/**
+ * Converts ts/tsx into js/jsx
+ * @param {string} file
+ */
+ const getImportWithConvertedExtension = (file) => {
+  if (file.indexOf('.') > -1) {
+    const fileName = file.substring(0, file.indexOf('.'))
+    const extension = file.substring(fileName.length + 1)
+
+    if (extension === 'ts')
+      return fileName + '.js'
+
+    if (extension === 'tsx')
+      return fileName + '.jsx'
+  }
+
+  return file
+}
+
 /**
  * Assuming an alias like this
  * ```
@@ -87,8 +106,8 @@ const resolvePath = (sourceFile, importPath, basePath, extensions, aliases, rela
     if (realFile) {
       return (
         relative
-          ? getRelativePath(sourceFile, realFile)
-          : realFile
+          ? getRelativePath(sourceFile, getImportWithConvertedExtension(realFile))
+          : getImportWithConvertedExtension(realFile)
       ).replace(/\\/g, '/')
     }
   }
NebraskaCoder commented 2 years ago

Any update on getting this to a stable release? I had to revert back to 1.0.3-rc.1 as 1.0.3 didn't have the fix and it is stopping my production build.