Skip to content

Restore open:* scripts #9

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.

Sign up for GitHub

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

Open
GrantASL19 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Restore open:* scripts #9

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6a1bc2f
Restore open:* scripts
GrantASL19 Jul 11, 2025
0dae4e7
Merge branch 'main' into feature/restore-open-scripts
daniellacosse Jul 17, 2025
06946a3
Merge branch 'main' into feature/restore-open-scripts
daniellacosse Sep 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
"build": "node ./wrapper_app_project/.scripts/build.mjs",
"clean": "npx rimraf node_modules/ output/",
"doctor": "./doctor",
"open:android": "cd $(find output -mindepth 1 -maxdepth 1 -type d ! -name 'mobileproxy' | head -n 1) && npm run open:android",
"open:ios": "cd $(find output -mindepth 1 -maxdepth 1 -type d ! -name 'mobileproxy' | head -n 1) && npm run open:ios",
Copy link
Collaborator

@daniellacosse daniellacosse Jul 17, 2025

Choose a reason for hiding this comment

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

"output" might be configured to something else so we can't rely on it.

I also think you mean npx cap open android|ios?

Copy link
Collaborator Author

@GrantASL19 GrantASL19 Jul 18, 2025

Choose a reason for hiding this comment

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

"output" might be configured to something else so we can't rely on it.

Right, I didn’t realize this until I worked on 75355ef.

This is tricky though — should we parse the config file in these scripts to determine where the project will be? But even if we do this the directory may not be in the config file if it was passed as a build command argument.

And thinking about this more we may have to since there could be multiple project directories in output, rendering the approach I have here a non-starter.

IMO this might not be worth the trouble at all since this is just a convenience script — what do you think?

I also think you mean npx cap open android|ios?

The generated project includes open:ios and open:android scripts, which were used in the previous implementation. npx cap open android|ios will also work if you prefer.

Copy link
Collaborator

@daniellacosse daniellacosse Jul 23, 2025

Choose a reason for hiding this comment

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

The generated project includes open:ios and open:android scripts, which were used in the previous implementation.

Ah right right.

I think the way to proceed here, annoyingly, would be to to write a script that takes a --output or --projectDir flag, that defaults to the output default.

Copy link
Collaborator Author

@GrantASL19 GrantASL19 Jul 23, 2025

Choose a reason for hiding this comment

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

I think the way to proceed here, annoyingly, would be to to write a script that takes a --output or --projectDir flag, that defaults to the output default.

What if we add an optional --openProject argument to the build command? So if you wanted to open the project after the build you would run:

npm run build -- --platform=android --entryUrl="https://jigsaw.google.com" --openProject

The build command already knows the output directory, and the obvious use case for the open:* scripts would be opening after build.

Either way I’ll look at this after #8 is merged since any work now is going to cause merge conflicts.

Copy link
Collaborator

@daniellacosse daniellacosse Jul 23, 2025

Choose a reason for hiding this comment

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

Sounds good to me! I'll look at #8 again shortly.

"reset": "npm run clean && npm ci",
"start:navigator": "node ./basic_navigator_example/.scripts/start.mjs"
},
Expand Down