Skip to content
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

Missing file extension on the import statement #622

Closed
DrSensor opened this issue Dec 27, 2020 · 9 comments
Closed

Missing file extension on the import statement #622

DrSensor opened this issue Dec 27, 2020 · 9 comments

Comments

@DrSensor
Copy link

DrSensor commented Dec 27, 2020

I just noticed that when building a .ts file, the import statement in the output doesn't include outExtension.

Assuming that "type": "module"

#!/usr/bin/env node
import { add } from './index'

console.log(add(1, 2))

👆 src/main.ts


require('esbuild').build({
    outdir: 'dist',
    entryPoints: ['src/main.ts', 'src/index.ts'],
    outExtension: { '.js': '.mjs' },
    minify: true,
})

👇 dist/main.mjs

#!/usr/bin/env node
import{add as o}from"./index";console.log(o(1,2));

As you notice it's importing from './index' which causes ERR_MODULE_NOT_FOUND when running dist/main.mjs.
According to nodejs docs, import keyword should have file extensions.

@DrSensor DrSensor changed the title Add file extension on the import statement Missing file extension on the import statement Dec 29, 2020
@evanw
Copy link
Owner

evanw commented Dec 31, 2020

The outExtension setting is for the extension of the generated output files, not for any input files they reference. So it's expected that this setting doesn't modify import paths.

For better or worse, the TypeScript team has decided that transforming TypeScript to JavaScript does not involve modifying the input file paths. If the environment you are running the code in does not implicitly add an extension, the way to do this with TypeScript is to write the desired file extension in the source itself. So to import ./index.js you would just write import {add} from './index.js' in your TypeScript code and the TypeScript type checker will import from ./index.ts instead of from ./index.js.

Unfortunately this doesn't work with the .mjs extension (I just tried it). The TypeScript type checker is unable to find ./index.ts if the import path is ./index.mjs. From the number of .mjs-related issues on their issue tracker it looks like Microsoft has been deliberately not adding support for modern node.js code. I'm not sure why since first-class modules in node are no longer under an experimental flag. I think the most relevant issue in this case is microsoft/TypeScript#27957.

I'm guessing the way people have been working around this in the meantime is to just not use the .mjs extension. The "type" field in package.json sets the meaning of the .js extension for your package. If you have "type": "module" in your package.json file, then .js and .mjs mean the same thing (and if you don't, .js and .cjs mean the same thing). So I think you should be able to just put "type": "module" in your package.json file and then use import { add } from './index.js' in your TypeScript file. Does that work for you?

@evanw
Copy link
Owner

evanw commented Jan 28, 2021

I'm going to close this because I believe the workaround mentioned above should work. You can also automatically add this extension with a plugin if you'd like (you have to enable bundling and then mark every import as external):

bundle: true,
format: 'esm',
plugins: [{
  name: 'add-mjs',
  setup(build) {
    build.onResolve({ filter: /.*/ }, args => {
      if (args.importer)
        return { path: args.path + '.mjs', external: true }
    })
  },
}],

@eric-burel
Copy link

Adding the .js extension explicitely in the .ts files does work, but may lead to other issues eg with Storybook: storybookjs/storybook#15962

The plugin you propose doesn't work in this scenario because you don't want to bundle the "index" file that exposes the ".js" esm. I get errors like this:

error: Plugin "add-js-ext" returned a non-absolute path: ./components/form/inputs/index.ts.js (set a namespace if this is not a file path)

It doesn't seem to do what I am looking for, adding ".js" to the "import { Foo } from "./Foo";

So so far the fix seems to be at Storybook level, adding a Webpack config to support ".js" in ".ts files.

@evanw
Copy link
Owner

evanw commented Sep 29, 2022

The plugin you propose doesn't work in this scenario because you don't want to bundle the "index" file that exposes the ".js" esm. I get errors like this:

error: Plugin "add-js-ext" returned a non-absolute path: ./components/form/inputs/index.ts.js (set a namespace if this is not a file path)

This would only happen if you deleted the , external: true part of the plugin. This part is important because it tells esbuild to use the returned path as-is and to not try to bundle it.

Adding the .js extension explicitely in the .ts files does work, but may lead to other issues eg with Storybook: storybookjs/storybook#15962

This is Storybook's problem, not esbuild's. The TypeScript team has explicitly decided that this is how TypeScript works. TypeScript takes a philosophical stance that your import paths in TypeScript code are supposed to be written in their final form, and are not supposed to be transformed when converting TypeScript to JavaScript.

So if you are compiling TypeScript to ESM files ending in .js, you'll need to use the .js extension in your TypeScript source code (because node requires explicit .js extensions for ESM code). Tools like Storybook that operate on TypeScript files will need to look for the corresponding .ts files when they see a .js import because that's a fundamental part of how TypeScript works.

@eric-burel
Copy link

Thanks so it confirms that the fix proposed at Storybook level, with resolve-typescript-plugin, is the most appropriate solution. I haven't tested with Jest or other bundlers yet, but at least it works ok in my app with Storybook during development.
I don't have problems when importing the built library either, it's an issue only during dev.

@stefanvonderkrone
Copy link

Hi here,
if I might suggest my (hacky) solution:
I use sed to add the .js extension to relative imports. I wasn't happy with adding --experimental-specifier-resolution=node to my node process. So here is what I do:

# let's find the end of an import statement,
# split the finding into two capture groups,
# combine the two catpure groups and insert '.js'
# there are three capture groups in fact, but the second one (\.\/|\.\.\/)
# is only used for matching the input, not for changing the output
SED_REGEX='s/([\} ]{0,1} from "(\.\/|\.\.\/){1,}[A-Za-z0-9\/]*)(";)/\1.js\3/'

# find all js files within our compiled server code
# and change the relative imports
find [build-output-folder] -type f -name '*.js' -exec sed -ri "$SED_REGEX" {} +

For me, it just works, so I'm happy for now. Dynamic imports are currently missing, so there is that.

@ds300
Copy link

ds300 commented Mar 22, 2023

We found the onResolve with external: true method produced some extremely funky, unreliable output, so after esbuild finishes we use recast (the codemod lib that jscodeshift is built on) to process all relative paths in imports and exports, adding the extension of the actual resolved files. So it should work with .mjs, .cjs, or plain ol' .js.

Just point it at a directory and it should do the right thing.

https://gist.github.com/ds300/f6177171ac673f98f6028799563a06db

@Neo-Ciber94
Copy link

Hi here, if I might suggest my (hacky) solution: I use sed to add the .js extension to relative imports. I wasn't happy with adding --experimental-specifier-resolution=node to my node process. So here is what I do:

# let's find the end of an import statement,
# split the finding into two capture groups,
# combine the two catpure groups and insert '.js'
# there are three capture groups in fact, but the second one (\.\/|\.\.\/)
# is only used for matching the input, not for changing the output
SED_REGEX='s/([\} ]{0,1} from "(\.\/|\.\.\/){1,}[A-Za-z0-9\/]*)(";)/\1.js\3/'

# find all js files within our compiled server code
# and change the relative imports
find [build-output-folder] -type f -name '*.js' -exec sed -ri "$SED_REGEX" {} +

For me, it just works, so I'm happy for now. Dynamic imports are currently missing, so there is that.

I ended doing something similar, but I'm not happy using a hack.

I like esbuild but for this I will need to drop it for rollup

@iamchathu
Copy link

What is we have the library as commonjs and distribute as commonjs and esm both?

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

No branches or pull requests

7 participants