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

Modify CheckEnv.js to notify dev if Vite keys are missing from .env file #1824

Open
3 of 5 tasks
Tracked by #1768
aqandrew opened this issue Sep 10, 2024 · 7 comments · May be fixed by #1822
Open
3 of 5 tasks
Tracked by #1768

Modify CheckEnv.js to notify dev if Vite keys are missing from .env file #1824

aqandrew opened this issue Sep 10, 2024 · 7 comments · May be fixed by #1822
Assignees
Labels
Feature: Code Health Make our code more readable, testable, and modular ready for dev lead ready for developer lead to review the issue Role: Frontend React front end work size: 0.50pt Can be done in 3 hours vite migration

Comments

@aqandrew
Copy link
Member

aqandrew commented Sep 10, 2024

Overview

We need to modify our check-env script to notify the dev if they are using non-vite compatible environment variables so that we can ensure previous devs have migrated to Vite.

Action Items

  • check keys with checkEnv.js and determine if VITE_ prefix exists, add the prefix if it is not present
  • notify the dev if the prefix was added

Resources

Info about relevant files

  • in package.json
    • start command: "start": "npm run check-env && npm run dev",
    • check-env command: "node ./utils/checkEnv"
  • checkEnv.js
    • utils/checkEnv.js

Archived Info

Details

Overview

This change is a simple variable renaming, which must be done for everyone running 311-Data after we switch from webpack to Vite. It could be done manually by inserting VITE_ at the beginning of their .env file. Or we could automate it, for example by modifying checkEnv.js.

Additionally, the repository secret must be updated so that the Mapbox API key is available in GitHub Actions. See #1778 (comment)

Action Items

Resources/Instructions

None

@aqandrew aqandrew added Role: Frontend React front end work Size: Missing Feature: Code Health Make our code more readable, testable, and modular draft Milestone: X - Technical Debt labels Sep 10, 2024
@ryanfchase ryanfchase added this to the X - Technical Debt milestone Sep 12, 2024
@ryanfchase ryanfchase added the ready for dev lead ready for developer lead to review the issue label Sep 12, 2024
@traycn traycn self-assigned this Sep 13, 2024
@traycn traycn linked a pull request Sep 13, 2024 that will close this issue
4 tasks
@traycn
Copy link
Member

traycn commented Sep 13, 2024

Reviewed PR #1822 dependency and it's looking great. Appreciate the clean up in the DbProvider.

@traycn
Copy link
Member

traycn commented Sep 13, 2024

For existing developers, modifying the checkEnv.js file sounds like a great idea. It'd be nice to take into account those previous devs who aren't aware of this change too.

For new developers, we can update the .example.env file and pre-pend VITE_ for the env variables there. So that cp .example.env .env reflect these changes. Refering to the command in the (README.md) steps.

@aqandrew
Copy link
Member Author

aqandrew commented Sep 25, 2024

Updating .example.env is a great idea. I'll do that + update/test checkEnv.js and include them in the PR.

Since checkEnv.js isn't mentioned in the README, I'll add some instructions for it there as well.

@ryanfchase
Copy link
Member

@traycn @aqandrew I'm going to modify the ticket in a couple of different ways:

  1. dev leads are going to take care of this in a separate issue:
  • ensure that this variable is renamed for everyone (they should all modify .env file since this is not checked into git)
  1. regarding automation of checking VITE_ prefix, we will make a dev ticket to update checkEnv.js to throw error (e.g. "Vite prefix needed in .env file")
  • I'd like to change the title / overview of this ticket to accomplish this change

@ryanfchase ryanfchase changed the title share instructions for renaming the MAPBOX_TOKEN environment variable to VITE_MAPBOX_TOKEN Modify CheckEnv.js to notify dev if Vite keys are missing from .env file Sep 25, 2024
@ryanfchase
Copy link
Member

This ticket is now dedicated to modifying the CheckEnv.js file. Changes to .example.env will be done on this ticket's PR

@ryanfchase ryanfchase added size: 0.50pt Can be done in 3 hours draft and removed draft Size: Missing labels Sep 25, 2024
@ryanfchase ryanfchase assigned aqandrew and unassigned ryanfchase and traycn Sep 26, 2024
@ryanfchase
Copy link
Member

After discussing this ticket, we've decided this work should be done as part of the existing PR. @aqandrew will get started on this, the work needs to be completed before the PR is merged.

@ryanfchase
Copy link
Member

Moved to In Review since I've been re-requested on the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature: Code Health Make our code more readable, testable, and modular ready for dev lead ready for developer lead to review the issue Role: Frontend React front end work size: 0.50pt Can be done in 3 hours vite migration
Projects
Status: In Review
Development

Successfully merging a pull request may close this issue.

3 participants