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

--platform=node --format=esm should output explicit ".js" imports #1343

Closed
izelnakri opened this issue Jun 3, 2021 · 5 comments
Closed

--platform=node --format=esm should output explicit ".js" imports #1343

izelnakri opened this issue Jun 3, 2021 · 5 comments

Comments

@izelnakri
Copy link

Node.js doesnt support relative imports without extension when used on esm mode("type": "module") Thus I can't transpile my typescript files to node esm runtime with esbuild. Commonjs format is not an option since my test runner code expects files to be esm modules.

esbuild packages/@memserver/model/test/index.ts --platform=node --format=esm

outputs:

import "./default-attributes-test";
import "./delete-test";
import "./index-test";
import "./insert-test";
import "./query-test";
import "./relationship-id-test";
import "./relationship-uuid-test";
import "./reset-database-fixtures-test";
import "./serialize-test";
import "./update-test";

It should output:

import "./default-attributes-test.js";
import "./delete-test.js";
import "./index-test.js";
import "./insert-test.js";
import "./query-test.js";
import "./relationship-id-test.js";
import "./relationship-uuid-test.js";
import "./reset-database-fixtures-test.js";
import "./serialize-test.js";
import "./update-test.js";
@evanw
Copy link
Owner

evanw commented Jun 3, 2021

The solution that TypeScript has designed for this scenario is that you actually write the .js extension in your TypeScript source code. When type checking, the TypeScript compiler will implicitly convert the .js to .ts and load the types from the corresponding TypeScript file. More info is available in this thread: microsoft/TypeScript#16577 (comment).

So you should just change your source code to get this to work. Getting this to work shouldn’t be done by changing esbuild itself. The TypeScript compiler doesn’t do this so esbuild shouldn’t either.

Closing as “by design.”

@evanw evanw closed this as completed Jun 3, 2021
@izelnakri
Copy link
Author

izelnakri commented Jun 3, 2021

Hi Evan, thank you for your quick response.

I've read the link you've shared, also I've been reading up on TypeScript teams decisions on allowing or changing explicit extensions. I have to say I'm deeply disappointed by their decision and justifications, I think they are doing a great disservice to the web development community as a whole by insisting extensionless imports or not allowing explicit TS imports or not rewriting them on output.

As you suggested moving the references to '.js' works for esbuild and tools that utilize it internally(like the test runner I built called qunitx), however the same code gives errors when I run with webpack, it says it cant resolve the modules. I tried to disable resolving with bunch of options still couldnt get it to work in webpack. In other words, TS teams stubbornness on this issue contradicts with principle of least surprise and causes unintuitive workarounds for developers. They should simply "change JS semantics" by rewrite or add extensions and I think esbuild should do as well because this also contradicts principle of least surprise:

esbuild packages/@memserver/model/test/index.ts --platform=node --format=esm

I expect this command to transform/build to code to be runnable in node.js esm mode. Although I know esbuild doesnt do typechecks or runs the code, it should still build the code that passes typescript checks in accordance to node.js esm environment which expects relative imports to be explicit with its extension, in addition to deno runtime.

@evanw
Copy link
Owner

evanw commented Jun 4, 2021

I agree that the situation is unfortunate. Arguably the root of the problem is that node.js allowed implicit extensions to begin with, which is a decision they have come to regret because of the complexity and performance issues it causes.

Anyway, I believe esbuild should follow the TypeScript team’s decision here. The TypeScript team owns the decisions about how their language works, not me. If this is to be changed the change would have to come from them.

If your TypeScript setup for Webpack can’t compile certain TypeScript code that the official TypeScript compiler can then that’s a limitation with that Webpack setup, not a problem with esbuild. I assume you’re hitting this: TypeStrong/ts-loader#465. According to the Webpack team, ts-loader shouldn't modify path resolution since it's a loader but you can write another plugin yourself to do this in Webpack: TypeStrong/ts-loader#1110 (comment). Someone has already written one and it's linked at the bottom of that thread.

If you want to use esbuild for your code but don’t want to change your source code, you can add the .js suffix to your import paths automatically by writing your own esbuild plugin. Here’s a simple example:

require('esbuild').build({
  entryPoints: ['packages/@memserver/model/test/index.ts'],
  bundle: true,
  platform: 'node',
  format: 'esm',
  plugins: [{
    name: 'resolve-ts',
    setup(build) {
      build.onResolve({ filter: /.*/ }, args => {
        if (args.kind === 'entry-point') return
        let path = args.path
        if (!path.endsWith('.js')) path += '.js'
        return { path, external: true }
      })
    },
  }],
})

Note that you need to include bundle: true for esbuild to run path resolution. Be aware that in general, path resolution rules are more complicated than just adding a .js suffix. The specific rules depend on the environment with which the code is expected to be compatible. The potential for needing to append /index.js instead of .js is one such edge case from the previously-linked thread on the TypeScript issue tracker. There may also be implicit .json extensions or other random things, especially if you’re working on a customized Webpack code base. The esbuild plugin API should be flexible enough to allow you to write an esbuild plugin to meet whatever custom path modification needs you have.

@izelnakri
Copy link
Author

izelnakri commented Jun 7, 2021

Hi Evan thank you for your detailed answer! I agree that its fair to not allow .ts if typescript doesnt allow it.

However esbuild should still transpile relative imports without extensions to explicit extensions when --mode=esm --platform=node selected because node wont execute the transpiled output and crash since it always expects explicit paths.

Since this is also the default browser behavior when script executed without --bundle on esbuild I still thing esbuild should implement the transpiled explicit extension output when received input has non explicit/implicit relative paths.

You've mentioned the topic of ts explicit resolved code as input during transpilation, which is another additional topic I think and I agree addons/plugins are probably a better place for this i.e a deno addon etc.

@djcsdy
Copy link

djcsdy commented Jul 7, 2021

@izelnakri (and others who may encounter this) in case you didn't see it in the linked issue, the plugin for webpack that handles this is here: https://github.com/softwareventures/resolve-typescript-plugin

Sorry to hijack the thread. Hopefully this helps someone.

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

3 participants