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: uncomment NEXT_PUBLIC_URL and NODE_TLS_REJECT_UNAUTHORIZED in the generated .env #73

Merged
merged 7 commits into from
Feb 13, 2025

Conversation

Ta5r
Copy link
Contributor

@Ta5r Ta5r commented Feb 12, 2025

What

This PR updates the .env file generation logic to ensure that NEXT_PUBLIC_URL and NODE_TLS_REJECT_UNAUTHORIZED are always included as an active (uncommented) entry. It also updates the corresponding test cases to reflect this change.

Why

Previously, NEXT_PUBLIC_URL and NODE_TLS_REJECT_UNAUTHORIZED were being commented out when they had a default value. Since this variables are crucial for frontend applications, ensuring it is always uncommented eliminates unnecessary setup issues.

Related Issue(s):

How

  • Updated access-functions.php: Modified the response of snapwp_helper_get_env_variables to include values to be passed to the reuired variables.
  • Updated Generator.php: Modified the condition in prepare_variable() to prevent 0 (zero) value from throwing an InvalidArgumentException.
  • Updated tests:
    • GeneratorTest.php
    • RestControllerTest.php

Screenshots

Screenshot 2025-02-13 at 1 00 51 PM

Checklist

  • I have read the Contribution Guidelines.
  • My code is tested to the best of my abilities.
  • My code passes all lints (PHPCS, PHPStan, ESLint, etc.).
  • My code has detailed inline documentation.
  • I have added unit tests to verify the code works as intended.
  • I have updated the project documentation accordingly.

@Ta5r Ta5r marked this pull request as ready for review February 12, 2025 12:31
@Ta5r Ta5r marked this pull request as draft February 12, 2025 12:32
@Ta5r Ta5r force-pushed the chore/uncomment-NEXT_PUBLIC_URL branch from 7305e84 to 8a7307a Compare February 12, 2025 13:17
@Ta5r Ta5r changed the title Uncomment NEXT_PUBLIC_URL by default Ensure NEXT_PUBLIC_URL is not commented out in the generated .env. Feb 12, 2025
@Ta5r Ta5r marked this pull request as ready for review February 12, 2025 13:26
@Ta5r Ta5r requested a review from justlevine February 12, 2025 13:26
@Ta5r Ta5r self-assigned this Feb 12, 2025
@justlevine justlevine changed the title Ensure NEXT_PUBLIC_URL is not commented out in the generated .env. fix: uncomment NEXT_PUBLIC_URL in the generated .env Feb 12, 2025
Copy link
Collaborator

@justlevine justlevine left a comment

Choose a reason for hiding this comment

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

We should also update NODE_TLS_REJECT_UNAUTHORIZED so its uncommented + set to 0 in our access function.

@Ta5r Ta5r changed the title fix: uncomment NEXT_PUBLIC_URL in the generated .env fix: uncomment NEXT_PUBLIC_URL and NODE_TLS_REJECT_UNAUTHORIZED in the generated .env Feb 13, 2025
Copy link
Collaborator

@justlevine justlevine left a comment

Choose a reason for hiding this comment

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

Love it - just need to adjust some of the generated comments (and the matching tests)

src/Modules/EnvGenerator/VariableRegistry.php Outdated Show resolved Hide resolved
src/Modules/EnvGenerator/VariableRegistry.php Outdated Show resolved Hide resolved
src/Modules/EnvGenerator/Generator.php Show resolved Hide resolved
@justlevine justlevine merged commit 9ff47b9 into rtCamp:develop Feb 13, 2025
9 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