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

Support *FbtEnum.ts files #3

Merged
merged 1 commit into from
Dec 18, 2024
Merged

Conversation

alexandernanberg
Copy link
Collaborator

@alexandernanberg alexandernanberg commented Dec 13, 2024

Stacked on #2 (needed the automatic external detection)

Currently you cannot have .ts files as enums and you get this error

node:internal/modules/esm/get_format:218
  throw new ERR_UNKNOWN_FILE_EXTENSION(ext, filepath);
        ^

TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension ".ts" for /Users/alex/dev/fbt/example/src/example/Example$FbtEnum.ts

I'm not sure the approach I've taken here is optimal, we could potentially use ts-node or esbuild-register but didn't have much luck with using them in an ESM environment.

Now every enum file is transpiled using TS regardless of the extension, it might be wiser to only transpile .tsx? files. I can update to do that if you think it's a good idea

@cpojer
Copy link
Collaborator

cpojer commented Dec 16, 2024

Did you try --experimental-strip-types? See https://nodejs.org/en/learn/typescript/run-natively

I do not mind that it's experimental, since it won't be experimental for too long.

Otherwise, I would much prefer to use Babel to strip TypeScript syntax instead of typescript because the typescript package is large and we are already using it in other places like for common strings: https://github.com/nkzw-tech/fbtee/blob/main/packages/babel-plugin-fbtee/src/bin/FbtCollector.tsx#L65-L88

Copy link
Collaborator

@cpojer cpojer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above

@alexandernanberg
Copy link
Collaborator Author

No, but AFAICT we cannot enable a node.js flag without manually spawning a new process (or require consumers to pass a flag). But maybe I'm missing something?

Makes sense to use Babel instead of TS though. Updated the PR!

@cpojer
Copy link
Collaborator

cpojer commented Dec 17, 2024

We should totally be in control of how the bin files are invoked, see https://github.com/nkzw-tech/athena-crisis/blob/main/codegen/generate-actions.tsx#L1C1-L2C1

I assume you have to make the changes to the src file, pnpm build and then see if it works or not. Do you mind trying?

@alexandernanberg
Copy link
Collaborator Author

TIL!

From what I can tell it works great. There is a warning logged when running the command that it's experimental but no big deal!

@cpojer
Copy link
Collaborator

cpojer commented Dec 17, 2024

Just add --no-warnings. Since it's fbtee's cli, that's fine.

Use babel for transpilation
@alexandernanberg
Copy link
Collaborator Author

Done! 👍

@cpojer
Copy link
Collaborator

cpojer commented Dec 18, 2024

So elegant. Thank you!

@cpojer cpojer merged commit 889b939 into nkzw-tech:main Dec 18, 2024
1 check passed
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

Successfully merging this pull request may close these issues.

2 participants