search

toddmedema / electrify

Take Charge of the Power Market
http://electrifygame.com
MIT License
6 stars 2 forks source link

Re-add dynamic version #92

Closed n-hebert closed 2 months ago

n-hebert commented 2 months ago

Closes #81

toddmedema commented 2 months ago

@n-hebert just tried it on my end and the version is coming up blank — I think maybe you put that value into your env by hand?

I did some googling and it seems like we also need to update the package.json commands to inject the version into the environment, this worked for me:

    "start": "REACT_APP_VERSION=$npm_package_version react-scripts start",
    "build": "REACT_APP_VERSION=$npm_package_version react-scripts build",
n-hebert commented 2 months ago

@n-hebert just tried it on my end and the version is coming up blank — I think maybe you put that value into your env by hand?

I did some googling and it seems like we also need to update the package.json commands to inject the version into the environment, this worked for me:

    "start": "REACT_APP_VERSION=$npm_package_version react-scripts start",
    "build": "REACT_APP_VERSION=$npm_package_version react-scripts build",

Ah woops! On it!

n-hebert commented 2 months ago

Hmm, were you on the latest version? I can confirm there was not an accidental manual insertion, and it wouldn't need the injection as you show since the .env file gets picked up automatically.

At least on my side, there's no REACTAPP anything in my shell env and it picks up from the .env file. Might need to pair if you can't fetch and find the same!

toddmedema commented 2 months ago

Oooh, I missed the .env change. Hmm, I'm hoping to have this pull from package.json so the version is only in one place / easier to maintain

n-hebert commented 2 months ago

Oooh, I missed the .env change. Hmm, I'm hoping to have this pull from package.json so the version is only in one place / easier to maintain

~I agree a hundred percent. I see it as a next ticket to do; merge this one, and then open an issue to de-duplicate. Rationale: this one patches a user-visible change (hi-pri) the other is just tech debt (low-pri).~

~Do you know if there's a low-hanging fruit for this one? I think that they are such diverse interests that it's not possible to have one read from the other right now.~ ~Leveraging a build script to .env.local could be a bit flaky, but work, if the duplication isn't worth merging.~

FIXED

n-hebert commented 2 months ago

How about this commit, @toddmedema ? :smile: