oxc-project / oxc

⚓ A collection of JavaScript tools written in Rust.
https://oxc.rs
MIT License
12.22k stars 442 forks source link

transform: remove ".ts" extension in output #5395

Open lukeed opened 2 months ago

lukeed commented 2 months ago

Given that this is typically used for library/bundler transformations, the generated output shouldn't contain extensions in the output.

I suppose the jury can still be out in regards to JS extensions, but TS extensions should definitely not be there.

Input

// src/walk.ts
import { join } from 'node:path';
import * as walk from './walk.ts';
import type { Options } from './walk.ts';

export type { Options };

export function up(name: string, options?: Options): string | undefined {
  // ...
}

Output

// output/find.mjs
import { join } from "node:path";
import * as walk from "./walk.ts";
export function up(name, options) {
  // ...
}

// output/find.d.mts
import type { Options } from "./walk.ts";
export type { Options };
export declare function up(name: string, options?: Options): string | undefined;

Note that both the generate dts and JS still contains the walk.ts reference.

Desired

// output/find.mjs
import { join } from "node:path";
import * as walk from "./walk";
export function up(name, options) {
  // ...
}

// output/find.d.mts
import type { Options } from "./walk";
export type { Options };
export declare function up(name: string, options?: Options): string | undefined;

Because this particular case is a library, ./walk will be re-resolved to the correct "exports" condition when needed.

And arguably, any bundler (or even runtime) contexts that ingest an extension-less import will still be able to resolve the local file sibling.

Boshen commented 2 months ago

Hi Luke, are we talking about a feature request for the up coming transformer https://www.npmjs.com/package/oxc-transform or something else?

lukeed commented 2 months ago

Hey -- Yes, I'm using that package, but it's currently a bug and not a feature request. When transforming TS source to JS, the generated JS still has imports referencing *.ts files.

Boshen commented 2 months ago

For reproduction:

import { transform } from "oxc-transform";

let sourceText = `
import { join } from 'node:path';
import * as walk from './walk.ts';
import type { Options } from './walk.ts';

export type { Options };

export function up(name: string, options?: Options): string | undefined {
  // ...
}
`
console.log(transform("asdf.ts", sourceText, {
  typescript: {
    onlyRemoveTypeImports: true,
    declaration: true,
  }
}))
{
  sourceText: 'import { join } from "node:path";\n' +
    'import * as walk from "./walk.ts";\n' +
    'export function up(name, options) {}\n',
  declaration: 'import type { Options } from "./walk.ts";\n' +
    'export type { Options };\n' +
    'export declare function up(name: string, options?: Options): string | undefined;\n',
  errors: []
}
lukeed commented 2 months ago

(On an unrelated note, sourceText is kinda a funny name for generated output. Renaming it is obviously breaking, but I'd suggest something like contents or javascript/js because "source text" makes me think it's the TypeScript source.)

Boshen commented 2 months ago

@Dunqing apparently we need to port https://babel.dev/docs/babel-preset-typescript#rewriteimportextensions

import { transformSync } from "@babel/core";

let sourceText = `
import { join } from 'node:path';
import * as walk from './walk.ts';
import type { Options } from './walk.ts';

export type { Options };

export function up(name: string, options?: Options): string | undefined {
  // ...
}
`

console.log(
  transformSync(sourceText, {
    presets: [["@babel/preset-typescript", { onlyRemoveTypeImports:true, rewriteImportExtensions:true }]],
    filename: 'script.ts',
  })
)
  code: "import { join } from 'node:path';\n" +
    'import * as walk from "./walk.js";\n' +
    'export function up(name, options) {\n' +
    '  // ...\n' +
    '}',
Boshen commented 2 months ago

(On an unrelated note, sourceText is kinda a funny name for generated output. Renaming it is obviously breaking, but I'd suggest something like contents or javascript/js because "source text" makes me think it's the TypeScript source.)

We'll align everything with babel.

lukeed commented 2 months ago

That "./walk.js" output is still incorrect/invalid. There needs to be no extension.

Boshen commented 2 months ago

Interesting, I suppose we need to change the API to allow removal. I see people had to come up with more plugins to fix this problem https://www.npmjs.com/package/babel-plugin-replace-import-extension?activeTab=readme 🤯

Boshen commented 2 months ago

rewriteImportExtensions: 'rewrite' | 'remove' | false

lukeed commented 2 months ago

Well, babel predates TS and export conditions, which are the two main players in affecting path resolution. Babel also positions itself & wants to be the be-all, end-all... AKA, it assumes it's always the final build.

I didn't know that OXC is aligning with Babel. NGL that makes me really nervous

Boshen commented 2 months ago

I didn't know that OXC is aligning with Babel

Our goal is not 100% compat, but we are currently aligning with Babel 8 to bootstrap ourself, otherwise we'll never ship. (Babel 8 is never going to be shipped isn't it ...)

We can always bend the API to allow current and future use cases.

Boshen commented 2 months ago

The latest oxc-transform@v0.26.0 includes the option rewriteImportExtensions: 'remove', but it's not declaration yet.

In:

let ret = transform("asdf.ts", sourceText, {
  typescript: {
    onlyRemoveTypeImports: true,
    rewriteImportExtensions: 'remove',
    declaration: true,
  }
});

console.log('Transformed:')
console.log(ret.code);

console.log('Declaration:')
console.log(ret.declaration);

Out:

Transformed:
import { join } from "node:path";
import * as walk from "./walk";
export function up(name, options) {}

Declaration:
import type { Options } from "./walk.ts";
export type { Options };
export declare function up(name: string, options?: Options): string | undefined;
Boshen commented 2 months ago

Declarations shouldn't remove extensions right? Otherwise type resolution will fail.

lukeed commented 2 months ago

They should be removed. If only publishing a build/ directory with the JS and DTS, the dts points to nothing if retains the *.ts import

Dunqing commented 2 months ago

They should be removed. If only publishing a build/ directory with the JS and DTS, the DTS points to nothing if retains the *.ts import

I don't fully understand why we need to remove them, I have tried the above example in tsc, and our output is the same as tsc's.

We are looking at your build script https://github.com/lukeed/empathic/blob/efc67f9c7fc5d2b0a6d748eeabc8a45d35c0a799/scripts/build.ts


Updated:

After looking at your script, I thought I knew why you needed to remove them; The transformed .d.ts file is named *.d.mts. So those .d.ts files don't exist anymore.