Skip to content

[TS] Generated Typescript code has .js extension #7661

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

Closed
stephanemagnenat opened this issue Nov 23, 2022 · 18 comments
Closed

[TS] Generated Typescript code has .js extension #7661

stephanemagnenat opened this issue Nov 23, 2022 · 18 comments

Comments

@stephanemagnenat
Copy link

stephanemagnenat commented Nov 23, 2022

Using Flatbuffers 22.11.22, and doing:

flatc -o src/flatbuffers --ts --gen-all schemas/all.fbs

The generated .ts files have a .js extension, leading to imports like this:

export { Table } from './namespace/table.js';

while previously (tested with version 2.0.6) they properly correctly had no extension:

export { Table } from './namespace/table';

The generated files themselves properly end with .ts.

You'll find a minimal test case here:
https://github.com/stephanemagnenat/flatbuffers-6739

Just type make and look in src/all_generated.ts.

@bjornharrtell
Copy link
Collaborator

I'm afraid this is not so simple.

Imports with .js extensions in ts code are not "wrong" as far I know and some environments require it.

However, ts files with js extension are wrong.

Will need to investigate but there is a larger rework of TS/JS generation work in progress already in #7510 which might affect this.

@stephanemagnenat
Copy link
Author

stephanemagnenat commented Nov 23, 2022

Sorry I was not very clear, the key problem is that flatc generates ts files with .ts extension but writes import of files ending with .js in the generated ts files, which obviously do not exist.

@bjornharrtell
Copy link
Collaborator

@stephanemagnenat but that is common practice and required by many module loaders. I agree it's not an optimal situation but have a look at microsoft/TypeScript#40878 for some history. If you use modules in a browser directly for example it will require the full path of the module, see https://stackoverflow.com/questions/55251956/how-does-javascript-import-find-the-module-without-an-extension.

@stephanemagnenat
Copy link
Author

I see, thank you for the explanation!

@Leandros
Copy link

@bjornharrtell: Can this be made at least configurable? There are plenty of projects where this will start throwing errors because of the added extension.

@bjornharrtell
Copy link
Collaborator

@Leandros mm, I'm open to add it as an option if really desired but I'm also curious of why/when it is cause of problem because I don't quite understand it. It's usually the other way round, no extension work "sometimes".

@mvdschee
Copy link

mvdschee commented Dec 1, 2022

I don't have a great example why, but it has something to do with the module resolution it could be that for example your local development is using TypeScript to discover it's files which will use a different methode of finding the right extension and a plugin like Eslint or Jest (running on Node.js) might use Node's module resolution (this I have not checked) and would fail to get the .js file to resolve properly to a TS file.

But I agree having an option to force an extension (or no extension) would be welcome to resolve some possible errors while working on a project.

jmillan added a commit to versatica/mediasoup that referenced this issue Dec 12, 2022
flatbuffers imports files with just extension which makes Jest fail.

google/flatbuffers#7661
@Leandros
Copy link

But I agree having an option to force an extension (or no extension) would be welcome to resolve some possible errors while working on a project.

I support this.

@Leandros
Copy link

@bjornharrtell I'm importing the generated TypeScript files inside a TypeScript backend written in NestJS. In my case, tsc can't cope with imports with a .js.

Generally, in the JavaScript/TypeScript world, it's common to avoid specifying any extensions for importing. More info here: https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/extensions.md

@bjornharrtell
Copy link
Collaborator

I've been working with JavaScript and TypeScript in lots of different projects and contexts and haven't encountered any issue with specifying the full module path with extension (after transpilation), only the other way around. So my own interest in this is low. But I will not oppose a PR that adds an option to generate without extension, so feel free to contribute.

@maxburke
Copy link
Contributor

Since upgrading to flatc-22.9.29 our TS builds have been breaking because of this change. Previous versions didn't specify an extension and worked fine.

@github-actions
Copy link
Contributor

This issue is stale because it has been open 6 months with no activity. Please comment or label not-stale, or this will be closed in 14 days.

@github-actions github-actions bot added the stale label Jun 30, 2023
@maxburke
Copy link
Contributor

maxburke commented Jul 1, 2023

not stale

@github-actions github-actions bot removed the stale label Jul 1, 2023
@surdu
Copy link

surdu commented Oct 2, 2023

In some development environment, there are not TS code file emitted. For example, ts-node has it disabled by default. The same default is used in React projects, at least if you use it with tools like react-scripts.

So yes, having the .js extension in the imports, breaks the code in environments like this.

PS: I would be curios in which situation, you have a TS module A, in which you want to import TS module B, yet you import module B using a .js extension. That mekes no sense to me ....

@bjornharrtell
Copy link
Collaborator

Since #7748 was implemented you can opt out of generating with js extension. Closing.

@surdu
Copy link

surdu commented Oct 2, 2023

Nice, but there is a bug in the implementation. When you have an import in one of your schema, the ts file will still contain an export with a .js extension:

If I have the following schemas:

// color.fbs

enum Color:byte { Red = 0, Green, Blue = 2 }
// monster.fbs

include "color.fbs";

namespace Sample;

table Monster {
  color:Color = Blue; // Enum.
}

root_type Monster;

After running flatc --ts --ts-no-import-ext -o monsters_generated monster.fbs we get a file monster.ts with the following content:

// automatically generated by the FlatBuffers compiler, do not modify

export { Color } from './color';
export * as Sample from './sample.js';

@bjornharrtell
Copy link
Collaborator

@surdu can you please report that as a separate issue?

@surdu
Copy link

surdu commented Oct 2, 2023

Sure thing. Just created #8105

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