-
Notifications
You must be signed in to change notification settings - Fork 5
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
Added ESBuild for building app and bundling/minfiying assets #375
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
The base branch was changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@Thomas-Geraghty when cherry-picking this to our projects…
which, if any, of these bullet points do you like and want me to copy back upstream? |
Thanks for testing this out @ushkarev and getting it integrated into another project! I know @ReedSoftware has integrated it into one of their projects too so he might have some additional fixes 😄 Just to address your points:
One of the major changes we've implemented on some of my projects is building one single file for the whole express app, and then building a sourcemap for it. In the current template implementation, every file outputted for the app ends up with a bunch of ESM/cJS converting logic, building one output file means there's just one instance and our end result is much neater - with sourcemaps continuing to allow for the same level of debugging. I'd be happy to backport this if people are comfortable, we're using it on 3 different projects without any concerns. |
@Thomas-Geraghty from our side, the only thing we've done is add an extra entry to Agree with all other points. Would be nice to have the config as ts - webpack and others manage it, and there appear to be examples of others using |
i’ve made the first pass at the above in hmpps-template-typescript#389 and included your error-catching tweaks @Thomas-Geraghty |
Note: The base for this PR is a branch where I've moved the assets folder to /server/assets, probably not necessary in retrospect. It makes this build process a bit tidier :)