Supports Electron 19+ #398
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
As we need further investigation on Electron support, close the PR for now: awslabs/aws-crt-nodejs#476 |
xiazhvera
commented
Jan 23, 2024
| } | ||
| ], | ||
| "node_cmd": "npm start", | ||
| "timeout": 180 |
There was a problem hiding this comment.
Electron windows sample would failed with a error prompt, and hangs in the CI. Set a timeout here to fail the process in this case.
sbSteveK
reviewed
Jan 25, 2024
| The sample is built with typescript@5^ and Electron@19. Please note the SDK currently does not support Electron20+. | ||
| Node14 is recommended to run the sample. | ||
|
|
||
| The sample is built with typescript@5^. Node14 would be minimal Node version to run the sample. |
There was a problem hiding this comment.
Trivial: "Node14+ is required to run the sample."
sbSteveK
reviewed
Jan 25, 2024
| npm run start | ||
| ``` | ||
|
|
||
| ### Websockets | ||
| ### MQTT over Websockets with TLS |
sbSteveK
reviewed
Jan 25, 2024
| npm run start | ||
| ``` | ||
|
|
||
| ### Websockets | ||
| ### MQTT over Websockets with TLS | ||
|
|
||
| To Run this sample using Websockets, go to the `node/pub_sub_electron_node` folder. | ||
| 1. Setup your credential. You will need to set your AWS credentials in your environment variables or local files. See the [authorizing direct AWS](https://docs.aws.amazon.com/iot/latest/developerguide/authorizing-direct-aws.html) page for documentation on how to get the AWS credentials, which then you can set to the `AWS_ACCESS_KEY_ID`, `AWS_SECRET_ACCESS_KEY`, and `AWS_SESSION_TOKEN` environment variables. |
There was a problem hiding this comment.
Trivial: remove "Setup your credential." and replace with "Setup your AWS credentials in your environment variables or local files."?
sbSteveK
reviewed
Jan 25, 2024
| ### Warning: `objc[79765]: Class WebSwapCGLLayer is implemented in both ` ? | ||
| ## Package | ||
| Please refer to (Electron-tutorial-packaging)[https://www.electronjs.org/docs/latest/tutorial/tutorial-packaging] for packaging details. | ||
| As our sample is using typescript, you would need include the compiled js files while packaging. You can config the `package.json` or `forge.config.js` to set it. |
There was a problem hiding this comment.
Trivial: ", you will need to include the compiled js files while packaging." "You can configure the package.json or forge.config.js to set the output folder for the compiled files."
sbSteveK
reviewed
Jan 25, 2024
|
|
||
| More info: https://github.com/electron/electron/issues/33685 | ||
| ## Electron Q&A |
sbSteveK
reviewed
Jan 25, 2024
| More info: https://github.com/electron/electron/issues/33685 | ||
| ## Electron Q&A | ||
| ### Warning: `objc[79765]: Class WebSwapCGLLayer is implemented in both ... ` | ||
| This is an issue running Electron on MacOS. The API has a name duplication for "WebSwapCGLLayer". The warning should not affect your development. The issue is fixed by Electron in v22. |
There was a problem hiding this comment.
Trivial: "This is an issue when running Electron on MacOS. The API has a name duplication for "WebSwapCGLLayer". This warning should not affect your development and has been fixed in Electron v22."
Note: Is it "WebSwapCGLLayer" or "WebSwapCGLayer". Just checking whether it's a double "L" in the name.
sbSteveK
reviewed
Jan 25, 2024
|
|
||
| ### SyntaxError: Unexpected token '?' | ||
| Please check your dependency and Node version. If the error is not from your code, it is most likely your dependency is using a different version of node. As the nullish coalescing operator (??) is introduced in Node14, using Node14+ would help. | ||
|
|
||
| ### N-API call failed: napi_create_external_arraybuffer( env, data_buffer->buffer, data_buffer->len, s_finalize_external_binary_byte_buf, data_buffer, &napi_binary). | ||
| Electron removed support for `napi_create_external_arraybuffer` since Electron@20. You can find more information from the Electron community here: https://github.com/electron/electron/issues/35801. There is no solid solution for the issue right now. Our team is actively working on resolving it. | ||
| The issue should be fixed in release v1.19.1. |
There was a problem hiding this comment.
Trivial: "This issue has been fixed in aws-iot-device-sdk-js-v2 v1.19.1."
sbSteveK
reviewed
Jan 25, 2024
|
|
||
| ### SyntaxError: Unexpected token '?' | ||
| Please check your dependency and Node version. If the error is not from your code, it is most likely your dependency is using a different version of node. As the nullish coalescing operator (??) is introduced in Node14, using Node14+ would help. | ||
|
|
||
| ### N-API call failed: napi_create_external_arraybuffer( env, data_buffer->buffer, data_buffer->len, s_finalize_external_binary_byte_buf, data_buffer, &napi_binary). | ||
| Electron removed support for `napi_create_external_arraybuffer` since Electron@20. You can find more information from the Electron community here: https://github.com/electron/electron/issues/35801. There is no solid solution for the issue right now. Our team is actively working on resolving it. | ||
| The issue should be fixed in release v1.19.1. | ||
| Electron removed support for `napi_create_external_arraybuffer` since Electron@20. You can find more information from the Electron community here: https://github.com/electron/electron/issues/35801. |
There was a problem hiding this comment.
Trivial: "Electron removed support for napi_create_external_arraybuffer in Electron v20."
sbSteveK
reviewed
Jan 25, 2024
| ### Why does the SDK not support Electron@20+ | ||
| Same as the above question. | ||
| ### Electron Packager Instructions "Error: An unhandled rejection has occurred inside Forge: Error: ENAMETOOLONG: name too long, scandir" with recursive path copy | ||
| The Electron Forge has an issue while copying files with a relative library path. We could avoid it by getting rid of the local path for the dependency. |
There was a problem hiding this comment.
Trivial: "Electron Forge has an issue when copying files with a relative library path. We can avoid it by removing the local path for the dependency."
sbSteveK
reviewed
Jan 25, 2024
| "aws-iot-device-sdk-v2": "^1.19.1", | ||
| } | ||
| ``` | ||
| Meanwhile if you would like to package the sample with your local library, you can manually use electron-packager with `--ignore=electron-packager` to work around (Reference:https://github.com/electron/electron-packager/issues/396) |
There was a problem hiding this comment.
Trivial: remove "Meanwhile", "to work around" -> "as a workaround"
sbSteveK
reviewed
Jan 25, 2024
| The library `aws-iot-device-sdk-v2` depends on the native modules `aws-crt`. In Electron 4.x and higher, the symbols needed by native modules are exported by electron.exe instead of node.dll or node.exe. In order to load native modules on Windows, the library need to install a delay-load hook that triggers when the native module is loaded to redirect the reference to use the loading executable (electron.exe in this case). A windows delay load is required for the library. | ||
|
|
||
| *If you still see this error after v1.19.1* | ||
| The issue usually indicates you are using a library distribution different from your development environment. When you run npm install, the node modules will pull the build files unique to your operating system, your architectures and the Node version. This usually happens when npm failed to pull the library with your development environment. You would like to checkout the library distribution and make sure you are using the correct binary build. |
There was a problem hiding this comment.
Trivial: "You would like to checkout the library distribution" -> "Inspect the library distribution"
sbSteveK
reviewed
Jan 25, 2024
|
|
||
| ### Error "GPU process launch failed: error_code=18" | ||
| Electron bug: https://github.com/electron/electron/issues/32074 | ||
| There is no valid work around for now, could be disabled by `--no-sandbox`, while it might not be an option in prod. |
…into electron_p
…into electron_p
bretambrose
approved these changes
Feb 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #, if available:
Description of changes:
Electron prevented using node library in render process. Update the sample to avoid loading libraries in pre_load.ts
Depends on awslabs/aws-crt-nodejs#525
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.