favware / esbuild-plugin-file-path-extensions

An esbuild plugin to automatically insert file extensions in your built JavaScript files based on the specified target
MIT License
21 stars 2 forks source link

bug: File extension gets added to non-js asset imports #132

Closed willwill96 closed 1 month ago

willwill96 commented 1 month ago

Is there an existing issue for this?

Description of the bug

When using this plugin with files that import non-js assets, like css or json, the file extension is added to these imports which causes them to fail once published

Steps To Reproduce

  1. Pull down this repro: https://github.com/willwill96/esbuild-plugin-file-path-extensions-repro/tree/main
    • Note that the export.ts file has the following:
      import './index.css'
      import pkgJson from './package.json'
      export function test() {
      console.log('package name', pkgJson.name)
      }
  2. npm install
  3. npm run build
  4. Note that the css and json imports have .mjs appended to them on the dist/export.js file, like this:
    
    import "./index.css.mjs";
    import pkgJson from "./package.json.mjs";
    function test() {
    console.log("package name", pkgJson.name);
    }
    export {
    test
    };

### Expected behavior

I expect non-js asset imports to be unchanged.

### Screenshots

_No response_

### Additional context

I am hoping for one of two updates:
- Check for the existence of the file before updating the import. This is my preferred method, but I could see it being somewhat resource intensive for large projects.
- Add some sort of configurable safelist for which file imports should be left alone. Something like:
```ts
import { defineConfig } from 'tsup';
import { esbuildPluginFilePathExtensions } from 'esbuild-plugin-file-path-extensions';

export default await defineConfig({
  format: ['esm'],
  entry: ['**/*.ts', "!tsup.config.ts", "!node_modules/**"],
  outDir: './dist',
  bundle: true,
  esbuildPlugins: [esbuildPluginFilePathExtensions({
    // imports ending in these strings wouldn't get have ".mjs" added to them
    safelist: [".css", ".json"]
  })]
});

I like option 1 better, because it would handle edge cases better. For example if someone decided to create a file called file.css.ts, you would still want the file extension added to an import like this: import something from './file.css'

willwill96 commented 1 month ago

I put up an PR here with the changes I would propose