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

FIX: Path vars for Windows environment #229

Merged
merged 12 commits into from
Sep 13, 2023
Merged

FIX: Path vars for Windows environment #229

merged 12 commits into from
Sep 13, 2023

Conversation

mmurrell-r7
Copy link
Contributor

@mmurrell-r7 mmurrell-r7 commented Sep 7, 2023

The previous released version does not execute properly in Windows machines. This PR implements necessary fixes to allow for an operational Windows installer and executable.

Bug:

  • On start-up, the Electron BrowserWindow is initialized, rendering the built html, css, and js assets. The filepaths for these assets are passed to our custom awsaml:// protocol handler, which opens and renders the files. On Windows machines, these filepaths will most likely live in the C:/ drive. When the filepath is passed to our protocol handler as a URL - e.g. awsaml://C:/Users/theUser/Downloads/awsaml/build/index.html - the C drive's colon : is removed and the filepath is handled as awsaml://C/Users/theUser/Downloads/awsaml/build/index.html which causes an error due to the filepath not existing. The BrowserWindow is unable to render properly.

Fix:
baseUrl = new URL(`awsaml:///${path.join(__dirname, '/../../build/index.html')}`).toString();

  • URL object normalizes the file path, which will fix issues with separator variation between OS (/ for POSIX, / or \ for Windows)
  • Converting the URL path to a string prevents the colon from being stripped out when passed to the protocol handler

@mmurrell-r7 mmurrell-r7 marked this pull request as ready for review September 8, 2023 15:34
anguyen-r7
anguyen-r7 previously approved these changes Sep 11, 2023
Copy link
Contributor

@anguyen-r7 anguyen-r7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

brew/cask/awsaml.rb Outdated Show resolved Hide resolved
@mmurrell-r7 mmurrell-r7 merged commit 7ac7e8e into master Sep 13, 2023
4 checks passed
@mmurrell-r7 mmurrell-r7 deleted the windows-build branch September 13, 2023 18:14
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.

3 participants