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

Port to TypeScript #191

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Conversation

erikpukinskis
Copy link

What:

Why:

How:

Checklist:

  • Documentation
  • Tests
  • Ready to be merged

@erikpukinskis
Copy link
Author

erikpukinskis commented Feb 13, 2023

Put some basic infra in place to build TypeScript.

Getting real type errors now, which is good progress:

Screen Shot 2023-02-12 at 6 25 16 PM

Going to be streaming the process on Twitch... Will probably be popping on and off quite a bit, since it's Sunday: https://www.twitch.tv/softwrengnr May take a bit to respond to chats.

@erikpukinskis
Copy link
Author

Alright, got from 113 problems down to 62:

Screen Shot 2023-02-12 at 9 31 19 PM

@conartist6
Copy link
Collaborator

I hate to be the bearer of bad news, but I have no idea if it is going to be possible to merge this.

@conartist6
Copy link
Collaborator

I'm the one who has been watching this repo most closely, but I'm really just keeping the lights on since @kentcdodds is busy with other things. I don't have the authority to accept such a change, and I'm not sure if I would accept it if I did.

@conartist6
Copy link
Collaborator

What is the argument for doing this? That is to say, what's the argument for rewriting the code instead of just providing good typedefs for external users? I understand that anyone who is used to writing typescript loves the idea, but it's impossible to escape the fact that it's a political idea: you welcome contributors who know typescript while rejecting those who do not. I've been following a similar (very heated) discussion over on eslint, and it seems to me that the major pieces of infra like eslint and babel have made a conscious decision to ensure that they are written in the language that everyone speaks.

@kentcdodds
Copy link
Owner

I'm fine with this, but people who work in the codebase should be the ones to decide and that's not me or drive-by contributors. It's @conartist6. So it's up to you, Conrad.

@conartist6
Copy link
Collaborator

Actually at the moment nobody works in the codebase regularly. I'm still working on facilitating the creation and use of macros, but I've moved on to pursue my own solutions in my own codebases.

@conartist6
Copy link
Collaborator

My hope is actually to replace the babel core with an API designed in such a way as to allow it to hit semver 1.0.0, and also to serve as the core for eslint and prettier.

@conartist6
Copy link
Collaborator

conartist6 commented Feb 13, 2023

I should clarify: the package babel-core is semver-stable, but I'm talking about the babel-traverse APIs -- the ones visible to transform authors and exposed through babel-plugin-macros. Those APIs including path are not semver-stable and cannot easily be made to be.

@erikpukinskis
Copy link
Author

erikpukinskis commented Feb 14, 2023

Thanks for the warning @conartist6! I totally understand if it ends up not being mergeable. It is—as you say—political. And obviously quite invasive.

My reason for doing it is that I am trying to write a macro for a project I'm working on (Codedocs). I wrote the macro at first in JavaScript, but I found it quite difficult to manage without types, so I tried porting it to TypeScript. Then I tried to write a test for the macro in TypeScript, but since createMacro in @types/babel-plugin-macros just returns any I was having a hard time grokking how to do that.

I'm building that project and tests in Vite, to add to the confusion. 😆 I just thought porting to TypeScript might make this all a bit understandable. If nothing else it will help me understand this package better!

To get a little into the political motivation, I just have a feeling that I can't really trust types unless the project itself is written in TypeScript. The fact that createMacro returns any is a perfect example! I want to know that the code and the types actually match up, especially in situations as complex as the babel-plugin-macros one, which is quite complex!

And lastly, a motivation for me is that I just love the concept behind babel-plugin-macros. It makes sense to me that library developers would own their compile-time scripts, rather than having app developer configure plugins for every package they use. I don't want this project to fade away, I'd love for it to thrive! Unless there's some alternative I'm not aware of, this project seems like a critical piece of infrastructure. And TypeScript I think is a sign of health (correctness anyway) in the JavaScript ecosystem.

Anyway... just some background information on why I'm doing this. But I totally understand if we end up not merging it.

@erikpukinskis
Copy link
Author

My hope is actually to replace the babel core with an API designed in such a way as to allow it to hit semver 1.0.0, and also to serve as the core for eslint and prettier.

That sounds really exciting! Would be amazing to have a stable API. Would that be a fork of @babel/traverse with semver guarantees?

@conartist6
Copy link
Collaborator

I'm glad you're interested in the project! I agree that macros are a great tool in the developer toolbox. I like that they make code transforms explicit in a way that I think should help make transformation much more approachable to novices. I think code transforms in general have a much greater role to play than they are currently playing, but for them to realize that potential we need to make them a whole lot nicer to use.

The stable API I'm working on isn't a fork of babel-traverse, but I am stealing any good ideas I can from there. I've also borrowed a lot of patterns from redux and redux-saga. I do intend to support basically everything that babel-traverse does, though I have no intention of creating direct compatibility. If anything I will use my completed APIs to offer codemods that help convert babel-traverse conventions to mine!

@erikpukinskis
Copy link
Author

@conartist6 That sounds amazing. And as someone who hates all of node_modules bloat that Babel seems to create, I would love for something simpler that does a useful subset of what Babel allows.

Are you thinking that might be babel-plugin-macros@4.0.0? Or a different package with a different scope?

@erikpukinskis
Copy link
Author

Update on the TypeScript: Finished my first pass at typing everything.

There are ~4 four errors remaining, which are all related to the return type of createMacro. Need to do some head scratching because I don't quite understand how you specify the default export in a macro?

Like in import styled from "@emotion/styled/macro"... I'm not exactly sure where the styled code comes from if the default export is the createMacro return value. Up to this point I've only ever done named exports in my macro! 🤔

Screen Shot 2023-02-13 at 11 00 47 PM

@conartist6
Copy link
Collaborator

It'll definitely be simpler. The core package will be cst-tokens and I'd imagine its relationship to babel-plugin-macros will depend on what its relationship to babel ends up being.

@conartist6
Copy link
Collaborator

Every individual macro will need typedefs, because all of them are going to need to invent lies about what types the variables they export are right?

@conartist6
Copy link
Collaborator

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.

None yet

3 participants