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

Fix TS transpile in CLI #1273

Merged
merged 8 commits into from
Aug 29, 2023
Merged

Fix TS transpile in CLI #1273

merged 8 commits into from
Aug 29, 2023

Conversation

frandiox
Copy link
Contributor

It looks like something changed in Remix and remixConfig.tsconfigPath now returns the jsconfig.json path when present instead of undefined like before (I think?). We were using that to detect if the project was TS or JS.

This PR fixes that issue and also replaces typescript with @babel/core + @babel/preset-typescript, which is 30MB smaller.

Copy link
Contributor

@blittle blittle left a comment

Choose a reason for hiding this comment

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

🙌

@wizardlyhel
Copy link
Contributor

Trying to understand the difference between typescript and babel package.

https://www.typescriptlang.org/docs/handbook/babel-with-typescript.html

In addition to that, Babel cannot create .d.ts files for your TypeScript which can make it harder to work with your project if it is a library.

Would this be a problem for us with our other library packages? (ie. hydrogen-react, hydrogen)

@frandiox
Copy link
Contributor Author

Would this be a problem for us with our other library packages? (ie. hydrogen-react, hydrogen)

No, Babel would only be used for project scaffolding and generating route files. E.g. you choose any combination of options in the init flow for a new project, we generate that in TS and then we transpile it to JS if that's what you chose. When doing that, we don't need to generate .d.ts for each generated .jsx, because we just want to drop types.

@frandiox
Copy link
Contributor Author

So at the end I'm reverting to TS again and just keeping this PR to fix the original issue and a minor refactor of unnecessary things we were doing.

The reason is that it doesn't seem like Babel installs faster than TS (2.8s Babel vs 1.8s TS in my tests) even though it's smaller. Probably related to the deep tree of dependencies it has.
It's perhaps still a viable option but since it doesn't improve much of what we already have (trade a few MB with speed), let's not fix something that isn't broken I guess.

@frandiox frandiox merged commit ee6e292 into main Aug 29, 2023
@frandiox frandiox deleted the fd-fix-ts-transpile branch August 29, 2023 02:29
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.

3 participants