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

ESM rewritten relative paths are wrong #2246

Closed
aubergene opened this issue May 12, 2022 · 8 comments
Closed

ESM rewritten relative paths are wrong #2246

aubergene opened this issue May 12, 2022 · 8 comments

Comments

@aubergene
Copy link

Given a structure like this

├── package.json
├── server.ts
└── src
    ├── index.ts
    ├── sub1
    │   └── index.ts
    └── sub2
        └── index.js

when I run the following

npx esbuild server.ts src/**/index.{ts,js} --format=esm --bundle --external:'./node_modules/*' --platform=node --target=node16.13 --outdir=dist --outbase=.

then the import paths are all rewritten to be relative as ../node_modules which is correct for the top level but it wrong for all levels below. I've tried reading the docs carefully and various incantations of outbase etc but haven't been able to get it to work. I'm also somewhat unclear as to why it's rewriting them at all, as presumably leaving external imports untouched would work?

In the dist dir I see the following

✅ correct

// server.ts
import { range } from "../node_modules/d3-array/src/index.js";
console.log(range(13));

🚫 broken

// src/index.ts
import { range } from "../node_modules/d3-array/src/index.js";
console.log(range(131));

🚫 broken

// src/sub1/index.ts
import { range } from "../node_modules/d3-array/src/index.js";
console.log(range(13));
@susiwen8
Copy link
Contributor

I think this is correct behaviour, cause esbuild will bundle all file into single file, the structure looks like

-dist
  -test.js
-node_modules
-src
 -test.ts
 -a
  -a.ts
 -b
  -b.ts

However I found potential bug for outbase, I have set outbase: '.', the output file located in dist/src/test.js , which is incorrect for node_modules path.

here is my build script

build({
    entryPoints: ['./src/test.ts'], 
    format: 'esm',
    external: ['./node_modules/*'],
    platform: "node",
    target: "node16.13",
    outdir: 'dist',
    outbase: '.',
    bundle: true
})

@aubergene
Copy link
Author

Thanks, yes, it feels like the expected behaviour should be the import path is relative to outbase for each file

@susiwen8
Copy link
Contributor

it feels like the expected behaviour should be the import path is relative to outbase for each file

I second that

@evanw
Copy link
Owner

evanw commented May 12, 2022

This general type of issue has been reported before: #1958 and #2164. I tried to support the "mark all packages as external" use case by reusing an existing mechanism (--external) but it didn't quite work, for various reasons. I'm planning to fix this by changing --external to just pass the paths through without any modification instead of turning them into relative paths at all. I haven't done that yet because it's a breaking change, but I still plan to do it. Can you confirm that this would work for your use case?

@aubergene
Copy link
Author

Yes thank you that would fix it! Also big thank you for all your time making esbuild!

@wonskarol
Copy link

I'm planning to fix this by changing --external to just pass the paths through without any modification instead of turning them into relative paths at all. I haven't done that yet because it's a breaking change, but I still plan to do it.

Do you know when you will release this version? I ask out of curiosity as we are considering using esbuild and it is currently blocking us.

@gmanninglive
Copy link

I'm planning to fix this by changing --external to just pass the paths through without any modification instead of turning them into relative paths at all. I haven't done that yet because it's a breaking change, but I still plan to do it.

Do you know when you will release this version? I ask out of curiosity as we are considering using esbuild and it is currently blocking us.

You've probably got something sorted now, but if not this plugin @evanw wrote fixes this for me.

Any imports that don't start with "/" or "./" or "../" are left alone

@malthe
Copy link

malthe commented Nov 2, 2022

@evanw not sure about that plugin, but it would be great if this just worked out of the box.

@evanw evanw closed this as completed in 7277ffd Dec 13, 2022
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

6 participants