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

Add missing functions required to succesfully connect with MySQL DB #1752

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

jeroenpf
Copy link
Contributor

Motivation for the change, related issues

When trying to connect to a MySQL database (in my case version 8.4.0) several errors regarding unreachable code were triggered, making the site unusable.

I've added the required functions one by one until there were no more errors and I was able to successfully run a site with MySQL.

The errors seem to be related to how MySQL handles authentication specifically.

Implementation details

I did not yet run a full build as I am not sure if there are any additional steps needed to do that. I would appreciate some help with this.

Testing Instructions (or ideally a Blueprint)

  • Run a MySQL database with Docker (use latest)
  • Remove SQLite integration plugin from WordPress Playground site and change wp-config.php to point to 127.0.0.1 and provide valid db credentials.
  • Observe that trying to use the MySQL db with a Playground site causes errors.
  • Build a new wasm binary and use it (and the accompanying js file) with the Playground site.
  • Observe that now the site works with MySQL.

@jeroenpf
Copy link
Contributor Author

@brandonpayton Do you have a chance to look at this?

@adamziel
Copy link
Collaborator

I did not yet run a full build as I am not sure if there are any additional steps needed to do that. I would appreciate some help with this.

The Dockerfile changes look great and there aren't any other changes required besides rebasing and building. See #1716 which did the same thing for another set of C functions.

@adamziel
Copy link
Collaborator

Also, does Electron support JSPI? I was thinking about shipping a set of JSPI binaries for runtimes that support it, it would solve this entire class of Asyncify problems.

@fluiddot
Copy link
Contributor

Also, does Electron support JSPI? I was thinking about shipping a set of JSPI binaries for runtimes that support it, it would solve this entire class of Asyncify problems.

I haven't specific documentation but I think it currently doesn't. However, Electron uses Chromium so if there's a way to enable that feature maybe we could give it a try.

@adamziel
Copy link
Collaborator

adamziel commented Sep 17, 2024

@fluiddot actually, how are you running Playground? Is it via the chromium JavaScript engine? If so, the chrome://flags/#enable-experimental-webassembly-jspi flag would do the trick. Or are you also shipping Node.js? If yes, can you ship Node v22? See the compat matrix: #1339

@fluiddot
Copy link
Contributor

@fluiddot actually, how are you running Playground? Is it via the chromium JavaScript engine? If so, the chrome://flags/#enable-experimental-webassembly-jspi flag would do the trick. Or are you also shipping Node.js? If yes, can you ship Node v22? See the compat matrix: #1339

@adamziel Ah, interesting. Playground is actually running via Node.js (version v20.17.0). I double-checked and Chromium is mainly used for rendering the frontend, and the JS engine is used for the view logic. As far as I know, Node.js is pre-bundled in Electron, so if we want to use a newer version we'd need to rebuild it. This will likely need to patch Electron due to potential incompatibilities. I'm not sure about the complexity fo this, but I presume it could be relatively high because the Node.js is a bit coupled to other dependencies in Electron like Chromium (example: electron/electron#43428).

@brandonpayton
Copy link
Member

The Dockerfile changes look great and there aren't any other changes required besides rebasing and building. See #1716 which did the same thing for another set of C functions.

@jeroenpf once this is rebased and the php-wasm binaries are rebuilt, this should be good to go.

You may already be familiar with building php-wasm, but just in case, you can do so with the following commands:

  • npm run rebuild:php:node
  • npm run rebuild:php:web

@jeroenpf
Copy link
Contributor Author

@fluiddot It looks like support for Node 22 will be added somewhere around November if we go by this chart.

@fluiddot
Copy link
Contributor

@fluiddot It looks like support for Node 22 will be added somewhere around November if we go by this chart.

Great! In that case, we could explore JSPI in a couple of months. I looked for a a potential alpha/beta build in Electron but looks like there's none with Node 22 yet.

@jeroenpf jeroenpf force-pushed the fix/mysql-asyncify-missing-functions branch from 7e6de42 to 3a16b4f Compare September 18, 2024 17:34
@jeroenpf
Copy link
Contributor Author

@adamziel - I've compiled new wasm binaries so we should be good to go :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Inbox
Development

Successfully merging this pull request may close these issues.

4 participants