JackDotJS / node-studio

Node-based music production tool, built with Tauri.
MIT License
9 stars 1 forks source link

Convert to TypeScript! #24

Closed bdotsamir closed 2 years ago

bdotsamir commented 2 years ago

will close #22

edit: this PR entirely changes the directory structure. from now on, all code belonging to the web will go in the web folder, and all code belonging to node.js will go in the node folder.

the way this works: we have three tsconfigs. one base tsconfig that gets called by the build script (tsc -b), then two "referenced" tsconfigs: one for the Web, and one for Node.

(source comment)

bdotsamir commented 2 years ago

please note that the script that create executables has been changed from "build" to "dist"

bdotsamir commented 2 years ago

paging jack: update the labels please <3

bdotsamir commented 2 years ago

rebased :man_dancing:

bdotsamir commented 2 years ago

looks like i duplicated some commits. not sure why. rebasing is always tricky.. i'm gonna try reverting then force pushing stale comment

bdotsamir commented 2 years ago

looks like i should be merging anyway, not rebasing, since it can cause issues (?) https://stackoverflow.com/questions/9264314/git-commits-are-duplicated-in-the-same-branch-after-doing-a-rebase

bdotsamir commented 2 years ago

branch updated

bdotsamir commented 2 years ago

right! here marks the end of the conversion from JS to TS. not time to celebrate yet though! still gotta check to see if electron can run it.

bdotsamir commented 2 years ago

image

well! almost there.

JackDotJS commented 2 years ago

image

wh... where's the title bar???

bdotsamir commented 2 years ago

probably got eaten by my window manager

JackDotJS commented 2 years ago

slightly spooky

is there a way to fix that

bdotsamir commented 2 years ago

it's a setting on my machine, not an issue with the code. not something fixable here since there's no bug to fix

JackDotJS commented 2 years ago

CANNOT REPRODUCE

ISSUE CLOSED

bdotsamir commented 2 years ago

Running into an issue where tsc will convert the entire project to either web-ESM or node-flavor CJS. There doesn't currently seem to be any way to convert only parts of the project to ESM or CJS.

Reference: https://github.com/electron/electron-quick-start-typescript/issues/27

I've tried utilizing tsconfig's references option to no avail

bdotsamir commented 2 years ago

tried webpack, doesnt seem to be playing nice. i expected that from webpack though

bdotsamir commented 2 years ago

update: i knew it had something to do with tsconfig references. i've just reproduced what this needed in a smaller, test program, and now i'll migrate it over to this project.

bdotsamir commented 2 years ago

update: i think i ran into a tsc bug :thinking: more to come

bdotsamir commented 2 years ago

i forgot to mention in this commit but this commit entirely changes the directory structure. from now on, all code belonging to the web will go in the dom folder, and all code belonging to node.js will go in the node folder.

the way this works: we have three tsconfigs. one base tsconfig that gets called by the build script (tsc -b), then two "referenced" tsconfigs: one for the DOM, and one for Node.

there is a common folder but I havent figured out what to do with it yet. that'll come in a later commit i guess

bdotsamir commented 2 years ago

the common folder is giving me hell all over again.

the web doesnt understand commonjs electron doesnt understand esmodules

https://github.com/electron/electron/issues/21457

bdotsamir commented 2 years ago

the next way i can think to solve this is actually to compile it as a node thing, expose it in the main world via the bridge, then add an entry to the index.d.ts file detailing the function signature so no type (generic) data is lost.

bdotsamir commented 2 years ago

the next way i can think to solve this is actually to compile it as a node thing, expose it in the main world via the bridge, then add an entry to the index.d.ts file detailing the function signature so no type (generic) data is lost.

This worked!

bdotsamir commented 2 years ago

This marks the ending of the conversion to TS!

What I was mainly worried about with this PR was making sure it would at least compile and that the modules would be understood by their respective sides (electron, web). Now that I've accomplished both of those tasks, @JackDotJS I'm ready when you are.

That said, there are still a few issues to be worked out, but those are in the JS/HTML realm- outside the scope of this PR. If you'd like me to squash those bugs in this PR then we can work on that together, but my two main goals have otherwise been met.

bdotsamir commented 2 years ago

Good morning gang.

This project has become a bit of a behemoth for me. I am now out of the house seven days a week; I am at university Monday through Friday, and I work Tuesday & Thursday evenings and Saturday & Sunday at unpredictable times. I am very much enjoying both of these things, but that means that projects like this have fallen by the wayside.

The only thing left that is blocking this PR from being merged is the isInterface() TS function. It's designed to reduce the amount of interface/type coercions we need to make and it generally makes the code look a bit cleaner and improves readability.

The problem lies in Electron and its lack of ESM support. I have tried 9 ways to Sunday to make the codebase as clean as possible and reduce the amount of code duplication I'd need to create in order to accomplish this, though thus far I've been unsuccessful. I've tried nearly everything; every package, every webpack configuration, everything short of publishing isInterface as an NPM package, or even forking Electron and building it with a version of Node that supports ESM out of the box. I'm not even sure either of these would work, and they're both quite drastic options. It's such a shame that Electron still does not support ESM, since that's where the Node.js ecosystem seems to be moving towards as a whole. A package that I love to use, chalk, has announced that it is dropping support for CJS in favor of ESM starting with its 5.x version. It will still maintain CJS support in 4.x, but there are no guarantees that it'll be supported for much longer since that is two entirely separate code bases to maintain.

The solution for now seems to be code duplication, unfortunately. When I'm able, I'll submit a last commit to this PR that copies the isInterface file into the respective opposite side, and it'll be good to merge.

This is not sustainable. If we want to have shared code between the DOM and Electron, we need to find a better solution (that's not context.exposeInMainWorld()), or Electron needs to get moving on ESM support, the latter being the more favorable of the two. ESM in Node has been stable since version 14-- that was two and a half years ago, with the conversation happening on Electron's end beginning a full year before that.

I'll see you all soon.

bdotsamir commented 2 years ago

These were committed in a class without testing. I'll test these when I get the chance- this PR is not technically ready though tsserver isnt yelling at me

bdotsamir commented 2 years ago

yknow. i have not yet tried swc.

it has a dedicated output for electron. https://swc.rs/docs/configuration/supported-browsers#targets

bdotsamir commented 2 years ago

update: swc is buggy at best. trying this: https://stackoverflow.com/questions/65380783/how-do-i-use-babel-with-electron

JackDotJS commented 2 years ago

electron makes me want to cry

bdotsamir commented 2 years ago

Still doing research on common code between frontend and backend. I want to make sure we have a solid foundation now before we have to deal with spaghetti code later on.

https://blog.logrocket.com/advanced-electron-js-architecture/#sharedmodule

JackDotJS commented 2 years ago

merging early while we begin working on stuff