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

Configure Vite to build the SDK #760

Merged
merged 3 commits into from
Dec 30, 2024
Merged

Conversation

sergei-maertens
Copy link
Member

@sergei-maertens sergei-maertens commented Dec 20, 2024

Depends on #758 to be merged first.

Should be a big milestone in #724 provided everything still works as expected

  • Set up Vite so it can produce the UMD bundle like CRA/webpack did/does
  • Set up Vite to produce a similar ESM build (but with less bugs than CRA/babel setup)

TODO

  • Properly test this

In a different PR we can then setup ESLint and swap out CRA with Vite 👀

Tested this locally with a form that had a file upload component, optional text fields and date picker, all seems to work as expected. I've also tested the appointment form flow and it also works as expected. Once Open Forms 3.0 is released, this should be good to merge!

Copy link

codecov bot commented Dec 20, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 80.53%. Comparing base (e473e32) to head (d179ef5).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
src/sdk.jsx 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #760      +/-   ##
==========================================
+ Coverage   80.10%   80.53%   +0.43%     
==========================================
  Files         238      238              
  Lines        4951     4953       +2     
  Branches     1348     1342       -6     
==========================================
+ Hits         3966     3989      +23     
+ Misses        955      933      -22     
- Partials       30       31       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sergei-maertens sergei-maertens force-pushed the chore/npm-package-to-vite branch from 643aa30 to 927dae1 Compare December 21, 2024 13:36
@sergei-maertens sergei-maertens marked this pull request as ready for review December 21, 2024 13:42
@sergei-maertens sergei-maertens force-pushed the chore/npm-package-to-vite branch from 927dae1 to 4bde5f9 Compare December 21, 2024 14:02
@sergei-maertens sergei-maertens marked this pull request as draft December 21, 2024 14:02
Slowly preparing to swap over the ESM build for the NPM package from
CRA/webpack to Vite.

This configuration was hand-crafted. Initially the library build config
was used/attempted (see
https://v5.vite.dev/config/build-options.html#build-lib), but this
proved not to be acceptable since it would inline all the font
assets, which blows up the CSS file size to unacceptable sizes.

This is very inefficient, since all the font assets would be loaded,
even if the browser only requires one type (woff2 vs ttf etc.). It
was also inlining the leaflet static images.

Using vitejs/vite#3295 (comment)
as a reference (which crucially doesn't define build.lib) allows us to
configure the underlying rollup setup without inlining assets.
Set up a build directory for vite configuration/plugins
to not clutter the vite.config.mts file too much.
@sergei-maertens sergei-maertens force-pushed the chore/npm-package-to-vite branch from 4bde5f9 to d179ef5 Compare December 30, 2024 08:21
@sergei-maertens sergei-maertens marked this pull request as ready for review December 30, 2024 08:21
@sergei-maertens sergei-maertens merged commit 0e16b0a into main Dec 30, 2024
14 of 19 checks passed
@sergei-maertens sergei-maertens deleted the chore/npm-package-to-vite branch December 30, 2024 08:42
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.

1 participant