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

WIP: BREAKING!: v3 #681

Closed
wants to merge 8 commits into from
Closed

WIP: BREAKING!: v3 #681

wants to merge 8 commits into from

Conversation

crookse
Copy link
Member

@crookse crookse commented Sep 30, 2022

Close #668

General Notes

  • Porting to Bun is trivial. See the README for benchmarks. The comparison to Deno is pretty insane; and the Deno Drash version uses URLPattern whereas the Node version (which is what the Bun version uses) uses the old way of handling resource paths. @ebebbington, remember when you added those regex patterns for optional params? Yea... the Bun implementation is using that stuff and it's fast.
  • I noticed some slow JS APIs in the Deno version. They are below:
    • Headers
    • URLPattern
  • We have "core" code and we have specific runtime code (e.g., src/core and src/node). This is because Node does not fully support the Web APIs (I could be wrong IDK...)
  • Oh... and this code is GNU GPL v3 now. Moving forward, we will use GNU GPL v3. This should be the case with all of our stuff.
  • Implementations of apps can be found in the benchmarks directory. We have Bun, Deno, and Node apps. Node apps come in ESM and CJS versions.
  • @Guergeiro, I tried to follow your chain of responsibility pattern as much as possible, but I diverted to improve speed. The RequestHandler and ResourceHandler classes build their own chains which run in Promise objects. Also, Drash is mostly async -- we return a Promise in most cases so as not to block anything. The only places where we really await are in the services code because those need to be executed in sequential order. Promise.all() will not work here because if you have 5 services for example, then the 5th service could change the response before the 1st service has a chance to act on it.
  • I should note some other stuff, but there is a lot going on in this PR. Let's bikeshed. Just kidding. Let's discuss though forreal if you (@ebebbington @Guergeiro @saragee3) have time. No pressure. Bun is still v0.

Node stuff

  • I got Node to work, but it required the core code using A LOT of generics. In theory, this is how reusable code should be done, but it doesn't feel quite clean. @Guergeiro, we rely on your expertise in design patterns here :D

@crookse crookse marked this pull request as draft September 30, 2022 23:39
@crookse crookse force-pushed the breaking/v3 branch 2 times, most recently from ae75391 to b788a30 Compare October 5, 2022 04:44
@crookse crookse closed this Jan 2, 2023
@crookse crookse deleted the breaking/v3 branch November 2, 2023 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

BREAKING!: v3-beta
1 participant