Skip to content

Update project using typescript and yarn for more comfortable usage #307

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
wants to merge 25 commits into from

Conversation

MDReal32
Copy link

No description provided.

@Julusian
Copy link
Collaborator

I don't know if you think this is ready to be looked at or not, seeing as it doesnt have a description.

I have a couple of thoughts from a quick look at the scope of the changes:

  • I have been tempted to convert this to typescript (or at least to classes syntax), but I don't really know the code, and worry that I would end up breaking things.
  • As this is such a wide reaching change, with the potential to break things, I am not sure I am comfortable putting it in 7.x. But I don't want to do a 8.x for years if possible.
  • What is the reason for using vite here instead of the typescript compiler? I've never felt a need to bundle serverside code, and vite doesnt do typechecking.

@MDReal32
Copy link
Author

MDReal32 commented Jul 25, 2023

  1. I have used vite for nothing. (It can be changed to default tsc compiler too.
  2. For breaking change I don't know to. But full code exactly copy of your code but in classes format. (At least i need that typing for my another project)
    And I added 1 more command for cmake install.

Release or not it depends on you. But if u don't want do it, can i publish it as seperate package in npm with reference to original.

@Julusian
Copy link
Collaborator

I have been working on a rewrite/overhaul in #348 which negates the need for this pr

@Julusian Julusian closed this Apr 20, 2025
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