post-intall script added #930
Conversation
|
|
desktop-app/package.json
Outdated
| "lint": "cross-env NODE_ENV=development eslint . --ext .js,.jsx,.ts,.tsx", | ||
| "package": "ts-node ./.erb/scripts/clean.js dist && yarn run build && electron-builder build --publish never", | ||
| "prepare": "cd .. && husky install desktop-app/.husky && chmod a+x desktop-app/.husky/pre-commit", | ||
| "prepare": "cd .. && husky install desktop-app/.husky && icacls desktop-app/.husky/pre-commit /grant Everyone:RX", |
There was a problem hiding this comment.
icacls doesn't seem to work on linux/mac. 🤔
There was a problem hiding this comment.
I apologize for any confusion. The "chmod" command is not recognized by Windows operating systems. So "icacls" is only for windows users. It think I shouldn't have put it in the merge request. It probably just requires a mention for contributors using windows.
There was a problem hiding this comment.
No worries, we just need to find a way that runs on all OSes.
desktop-app/package.json
Outdated
| @@ -27,10 +27,10 @@ | |||
| "build": "concurrently \"yarn run build:main\" \"yarn run build:renderer\"", | |||
| "build:main": "cross-env NODE_ENV=production TS_NODE_TRANSPILE_ONLY=true webpack --config ./.erb/configs/webpack.config.main.prod.ts", | |||
| "build:renderer": "cross-env NODE_ENV=production TS_NODE_TRANSPILE_ONLY=true webpack --config ./.erb/configs/webpack.config.renderer.prod.ts", | |||
| "postinstall": "yarn rimraf --glob node_modules/browser-sync/dist/**/*.map && yarn replace '\"network-throttle\".*' '' node_modules/browser-sync-ui/lib/UI.js && ts-node .erb/scripts/check-native-dep.js && replace-in-file '/// <reference types=\"howler\" />' \"import { Howl } from 'howler';\" node_modules/use-sound/dist/types.d.ts && electron-builder install-app-deps && cross-env NODE_ENV=development TS_NODE_TRANSPILE_ONLY=true webpack --config ./.erb/configs/webpack.config.renderer.dev.dll.ts", | |||
| "postinstall": "ts-node postinstall.ts", | |||
There was a problem hiding this comment.
Can we keep the app dependencies installation and webpack here only?
So that the postinstall.ts can only take care of the housekeeping stuff and at the same time the visibility of these important commands is maintained too.
There was a problem hiding this comment.
The updated commit seems to have added a duplicate entry for postinstall. Hope that's not intentional.
There was a problem hiding this comment.
Hi Manoj,
Thank you so much for the feedback. Please consider the latest changes.
There was a problem hiding this comment.
Looks good now, thank you for your patience!
There was a problem hiding this comment.
Will merge this after cutting a release. So, we'll have time to test and fix if there are any issues.
|
@all-contributors Please add @monalisamsteccentric for code. |
|
I've put up a pull request to add @monalisamsteccentric! 🎉 |
✨ Pull Request
📓 Referenced Issue
Fixes: #859
#859 (comment)
ℹ️ About the PR
I have made some modifications to the code to address the issue of the post-install script not working correctly on Windows. However, I will need assistance to determine whether the changes also work as expected on macOS.
To solve the problem on Windows, I added a postinstall.ts script. This script handles the correct parsing of commands, including the proper escaping of angled brackets, which was causing issues on Windows systems.
In addition to the postinstall.ts script, I made changes to the prepare script to improve the setup process. Here are the modifications I made:
"prepare": "cd .. && husky install desktop-app/.husky && chmod a+x desktop-app/.husky/pre-commit",
"prepare": "cd .. && husky install desktop-app/.husky && icacls desktop-app/.husky/pre-commit /grant Everyone:RX"
The first line of the prepare script remains the same as before, which installs Husky and sets up the pre-commit hook in the desktop-app/.husky directory.
The second line is an additional command I added to modify the permissions of the pre-commit file. On Windows, it uses the icacls command to grant read and execute permissions to everyone (Everyone:RX).
🖼️ Testing Scenarios / Screenshots
To ensure compatibility and proper execution, I conducted tests on Windows. Here are the steps I followed during testing:
Checked the permissions of the pre-commit file to confirm that it had the appropriate permissions (Everyone:RX on Windows).
Attached is a screenshot showing the successful execution of the updated prepare script on the Windows environment.
I believe these changes, along with the addition of the postinstall.ts script, resolve the issue mentioned in #859 and enhance the overall setup process.
I kindly request the community to review and merge this pull request. Please let me know if you have any questions or if further tests or modifications are required.
Thank you for your attention and assistance!