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: base url env & refactor [LIBS-635] #872

Merged
merged 7 commits into from
Aug 27, 2024
Merged

Conversation

KaiVandivier
Copy link
Contributor

@KaiVandivier KaiVandivier commented Aug 26, 2024

Part of LIBS-635

I noticed that the PUBLIC_URL and BASE_URL env vars weren't making it to built apps after some Vite redesigns -- the cause was that they were added onto process.env directly separate from other env var handling, which ended up happening after process.env was parsed and filtered for sending to the shell.

An easy fix would be to just move the process.env modification before where the shell env gets parsed, but much of the env handling was getting dispersed in several locations, and I thought this could be a good time to try to simplify things.

By making a standalone getEnv function, then bootstrapShell could be its own function, and then we don't need to keep track of a shell object any more, simplifying that concept in the script a bit 👍

I ended up leaving the baseUrl and publicUrl logic where it is for printing purposes, but tracked them as regular variables and only adding them to the env later with everything else

@KaiVandivier KaiVandivier requested review from amcgee and kabaros August 26, 2024 13:19
Copy link
Member

@amcgee amcgee left a comment

Choose a reason for hiding this comment

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

Much cleaner! Code looks good ✅

Looks like there are just a few leftover comments to clean up?

@KaiVandivier KaiVandivier merged commit 7f19259 into alpha Aug 27, 2024
8 checks passed
@KaiVandivier KaiVandivier deleted the fix-base-url-env branch August 27, 2024 13:19
dhis2-bot added a commit that referenced this pull request Aug 27, 2024
# [12.0.0-alpha.8](v12.0.0-alpha.7...v12.0.0-alpha.8) (2024-08-27)

### Bug Fixes

* base url env & refactor [LIBS-635] ([#872](#872)) ([7f19259](7f19259))
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 12.0.0-alpha.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

dhis2-bot added a commit that referenced this pull request Dec 13, 2024
# [12.0.0](v11.7.5...v12.0.0) (2024-12-13)

### Bug Fixes

* **deps:** upgrade app-runtime and ui ([#895](#895)) ([8ed0ec3](8ed0ec3))
* **deps:** upgrade react to 18 in example apps ([#900](#900)) ([7fd16d7](7fd16d7))
* **deps:** use npm v6 before publishing ([01ad502](01ad502))
* **init:** update bootstrap script branch ([#896](#896)) ([33c261a](33c261a))
* **plugin:** clean up resize observer and handle sonarqube warnings ([#898](#898)) ([f113dd5](f113dd5))
* alerts from plugins [LIBS-695] ([#881](#881)) ([21be0d2](21be0d2))
* allow serving files from cwd node_modules ([0233949](0233949))
* base url env & refactor [LIBS-635] ([#872](#872)) ([7f19259](7f19259))
* bump ui library ([#882](#882)) ([1ae9569](1ae9569))
* clear only required build dirs ([#894](#894)) ([179305f](179305f))
* env refactor for Vite wrap-up [LIBS-690] ([#889](#889)) ([84da4e6](84da4e6))
* injectPrecacheManifest warning logging ([a0d266e](a0d266e))
* normalize to .js extensions when compiling libraries ([#893](#893)) ([58b33a8](58b33a8))
* **service-worker:** handle undefined config vars in injectPrecacheManifest ([a90a4e0](a90a4e0))
* correct app shell paths ([#883](#883)) ([a1af71c](a1af71c))
* handle jsx in js support [LIBS-633] ([#871](#871)) ([595a35d](595a35d))
* increase precache max file size to 3 MB ([b20ed22](b20ed22))
* remove custom eslint from TS template ([71cef4b](71cef4b))
* update deps ([1e7ce93](1e7ce93))
* update workbox deps to avoid deprecation warnings ([9a81c4a](9a81c4a))
* use strings for 'boolean' env vars ([eaf5e66](eaf5e66))

### Features

* create initial TS template ([#868](#868)) ([2795f79](2795f79))
* enable HMR for .js files ([5f4683c](5f4683c))
* handle plugins with Vite [LIBS-610] ([#863](#863)) ([ca5be0d](ca5be0d))
* jsx migration script ([#869](#869)) ([7764f49](7764f49))
* migrate snap files too ([#878](#878)) ([521f483](521f483))
* replace CRA with Vite [LIBS-598] ([#847](#847)) ([3dd0e59](3dd0e59))
* upgrade react to v18 ([#876](#876)) ([77ecf10](77ecf10))
* **init:** set direction: 'auto' and import locales for new apps [LIBS-645] ([#867](#867)) ([4eda4a9](4eda4a9))

### BREAKING CHANGES

* require react version 18

* fix: pin react version to 18

* test: update test and test libraries for react 18
* Supported Node versions are 18.x or 20+

* ci: upgrade Node version

* fix: always add PWA_ENABLED to app env for better static analysis

* chore: pause precache manifest injection

* fix: building SW without CRA

* chore: comment update

* fix: group moment locales in their own dir

* refactor: clean up precache injection

* fix: locale utils and handling moment in jest

* fix: compile correctly after merging changes

* chore: comment in compile.js

* chore: some clean-up

* chore: comments

* fix: use port 3000 for the dev server

* fix: improve moment locale chunk naming

* chore: remove CRA

* fix: use mjs build of Vite

* fix: bump cli-style for CRA fix

* feat: use interactive dev server output from Vite

* fix: make dev server port configurable

* chore: remove old index.html

* fix: env tweaks
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 12.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

3 participants