Build demo app for distribution / preview #184
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
sadiqkhoja
force-pushed
the
features/ui/build-demo-app
branch
from
be7956f to
e10a1da
Compare
sadiqkhoja
force-pushed
the
features/ui/build-demo-app
branch
from
e10a1da to
ff7183e
Compare
eyelidlessness
added a commit
that referenced
this pull request
Sep 11, 2024
This is consistent with all of the other Web Forms packages. This merges the Vitest config as-is, into the Vite config from #184 with slight adjustments: - Give a name to express intent of branching on mode === ‘demo’ - Set up branchy config options early, assign to single return object of all combined config I think this makes it more clear what is conditional in the config, and why it should be conditional. Note: this _mostly satisfies_ our Vue/non-Vue build story. I would like to consider aligning the Vue/non-Vue approach with the Solid/non-Solid approach (see: packages/xforms-engine/vite.config.ts, and the env flag set in packages/xforms-engine/package.json scripts).
eyelidlessness
approved these changes
Sep 11, 2024
There was a problem hiding this comment.
This is good as-is, so you can feel free to merge if you prefer. But I have a branch with some related additive changes that I think would be good to bring in if you'd like to incorporate them: diff.
As with #183, I broke this down by commit-with-reasoning. The general goal of the changes are:
- Reduce differences between the
web-formsVite/Vitest configs and other packages (also fixes some confounding config type issues in the process) - Eliminate the internal build (for E2E testing, originally introduced in fix: include css in the built js file #152), since this new build now satisfies that need and it redundant
I did also make some slight changes to the branchy logic in the config, I think it makes the intent clearer. But the resulting config is unchanged.
Edit: and here's everything rebased if that helps.
eyelidlessness
added a commit
that referenced
this pull request
Sep 11, 2024
This is consistent with all of the other Web Forms packages. This merges the Vitest config as-is, into the Vite config from #184 with slight adjustments: - Give a name to express intent of branching on mode === ‘demo’ - Set up branchy config options early, assign to single return object of all combined config I think this makes it more clear what is conditional in the config, and why it should be conditional. Note: this _mostly satisfies_ our Vue/non-Vue build story. I would like to consider aligning the Vue/non-Vue approach with the Solid/non-Solid approach (see: packages/xforms-engine/vite.config.ts, and the env flag set in packages/xforms-engine/package.json scripts).
sadiqkhoja
pushed a commit
to sadiqkhoja/web-forms
that referenced
this pull request
Sep 12, 2024
This is consistent with all of the other Web Forms packages. This merges the Vitest config as-is, into the Vite config from getodk#184 with slight adjustments: - Give a name to express intent of branching on mode === ‘demo’ - Set up branchy config options early, assign to single return object of all combined config I think this makes it more clear what is conditional in the config, and why it should be conditional. Note: this _mostly satisfies_ our Vue/non-Vue build story. I would like to consider aligning the Vue/non-Vue approach with the Solid/non-Solid approach (see: packages/xforms-engine/vite.config.ts, and the env flag set in packages/xforms-engine/package.json scripts).
This (finally!) fixes very confusing TypeScript type errors caused by trying to use the `defineConfig` extension exported from `vitest/config`. In turn, it will allow cleanup and greater consistency across all of the Vite/Vitest configs for each package.
This is consistent with all of the other Web Forms packages. This merges the Vitest config as-is, into the Vite config from getodk#184 with slight adjustments: - Give a name to express intent of branching on mode === ‘demo’ - Set up branchy config options early, assign to single return object of all combined config I think this makes it more clear what is conditional in the config, and why it should be conditional. Note: this _mostly satisfies_ our Vue/non-Vue build story. I would like to consider aligning the Vue/non-Vue approach with the Solid/non-Solid approach (see: packages/xforms-engine/vite.config.ts, and the env flag set in packages/xforms-engine/package.json scripts).
This new build, intended for downstream consumption, is both suitable for the testing purpose and a better basis for it: it’ll test **a real build**!
This is made possible by Vitest using the same version/install of Vite as we use
sadiqkhoja
force-pushed
the
features/ui/build-demo-app
branch
from
643f9a7 to
67577b9
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This enables us to deploy demo app anywhere as a static web-site. For example: if you run
npx live-server packages/web-forms/dist-demoafter building the project, you will see the demo App.