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

feat(remix-dev/cli): add --debug-binding flag #4325

Closed
wants to merge 3 commits into from

Conversation

jstafman
Copy link
Contributor

@jstafman jstafman commented Oct 5, 2022

Closes: #

  • Docs
  • Tests

My preference would be to make --debug accept an optional value as described here: #4216 (comment). However, I was not able to figure how to make that work. I've tried making it a string instead of boolean, but it is rejected when no value is provided. I also tried creating a custom handler, with the same result. If anyone has a suggestion on how to approach that, please let me know. There is currently an open issue about this for the arg package: vercel/arg#61 (comment).

Testing Strategy:

  • Tested out different combinations of using the --debug-binding flag using a playground, including:
    • remix dev --debug (default)
    • remix dev --debug --debug-binding 0:0:0:0:9229
    • remix dev --debug --debug-binding 0:0:0:0
    • remix dev --debug-binding 0:0:0:0:9229 (shouldn't load debug at all)
  • Ran yarn test and all tests passed

@changeset-bot
Copy link

changeset-bot bot commented Oct 5, 2022

⚠️ No Changeset found

Latest commit: 1704f35

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@jstafman jstafman changed the title Allow custom debug binding feat(remix-dev/cli): add --debug-binding option Oct 5, 2022
@machour
Copy link
Collaborator

machour commented Jan 22, 2023

@jstafman can you rebased & fix the conflict? 🙏🏼

@jstafman
Copy link
Contributor Author

@jstafman can you rebased & fix the conflict? 🙏🏼

@machour - All set.

@MichaelDeBoey MichaelDeBoey changed the title feat(remix-dev/cli): add --debug-binding option feat(remix-dev/cli): add --debug-binding flag May 1, 2023
@MichaelDeBoey
Copy link
Member

Hi @jstafman!

Are you still interested in getting this one merged?

If so, please rebase onto latest dev & resolve conflicts

@MichaelDeBoey MichaelDeBoey added the needs-response We need a response from the original author about this issue/PR label May 1, 2023
@jstafman
Copy link
Contributor Author

jstafman commented May 2, 2023

Hi @jstafman!

Are you still interested in getting this one merged?

If so, please rebase onto latest dev & resolve conflicts

@MichaelDeBoey - It looks like a lot has changed since I first created this pull request. It might be easier for me to start with the current dev branch and redo my work from there (it wasn't a lot). Do you think this is something worth pursuing?

@github-actions github-actions bot removed the needs-response We need a response from the original author about this issue/PR label May 3, 2023
@pcattori
Copy link
Contributor

pcattori commented May 3, 2023

The new dev server (unstable_dev) runs alongside your app server. So if you wanted to add debugging to your app server, you can now do so directly without modifying the dev server.

Since we are actively pushing towards v2 right now and will be removing the old dev server flow, might be best to try this with the new dev server and only proceed if there are issues with that.

@pcattori
Copy link
Contributor

pcattori commented Jul 5, 2023

The new v2 dev server is now stabilized. With v2_dev enabled, you can call node directly for your server and pass in any flags without indirection.

@pcattori pcattori closed this Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants