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

build specific api files #226

Merged
merged 3 commits into from
Jan 22, 2024
Merged

build specific api files #226

merged 3 commits into from
Jan 22, 2024

Conversation

bumi
Copy link
Contributor

@bumi bumi commented Jan 20, 2024

based on the build this copies the request api that will be used in the JS app

based on the build this copies the request api that will be used in the JS app
@@ -5,9 +5,11 @@
"type": "module",
"scripts": {
"dev": "vite",
"dev:wails": "VITE_NWC_APP_TYPE=WAILS vite",
"dev:wails": "yarn prepare:wails && VITE_NWC_APP_TYPE=WAILS vite",
Copy link
Contributor

Choose a reason for hiding this comment

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

VITE_NWC_APP_TYPE should not be needed now, right?

Should we delete the original request.ts file and add it to .gitignore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if you think that's a doable way, we can go forward with that.

@rolznz rolznz changed the base branch from feat/single-user to feat/wails-v2 January 22, 2024 01:42
@rolznz
Copy link
Contributor

rolznz commented Jan 22, 2024

Downsides of this approach:

  • accidentally editing request.ts and it making no changes and being overwritten if the build or dev mode is restarted
  • hot reloading does not work for platform-specific files

But we probably will not make many more edits to this file.

The alternative would be a dynamic import using the value of an env variable (that was removed in this PR) as part of the filename of the imported file, which would not have the downsides mentioned above.

I will merge this for now, in the rare case that we end up adding more platform-specific code we could revisit it.

@rolznz rolznz merged commit 6c66bda into feat/wails-v2 Jan 22, 2024
0 of 2 checks passed
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.

2 participants