-
Notifications
You must be signed in to change notification settings - Fork 94
LF-4842 Copy all non cjs, js, ts files to compiled dist folder #3803
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
LF-4842 Copy all non cjs, js, ts files to compiled dist folder #3803
Conversation
navDhammu
left a comment
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.
This looks like a good working solution - by copying the non-js, ts files over to dist. My main concern is that it may include every type of file and not just the ones we need, like .env or other hidden files that may somehow cause unintended side-effects.
Also, I noticed the dist output includes /shared and /.knex folders as well, so if there are any needed non-TS or JS files in those directories, they would also need to be copied.
I'm thinking its better to just copy the exact files we need? One alternate and more long-term solution that I saw someone mention somewhere, (I think it was in this discussion) is to create a dedicated static folder that will include all template files and other static assets needed by the backend in /api/static, and copy it over to /dist/api/static during build.
|
Chatted a bit synchronously about this today, but updating here, too. I was looking at the files copied with rsync -avi --dry-run --exclude='*.js' --exclude='*.ts' --exclude='*.cjs' ./src/ ./dist/app/src/and personally I like the idea of only copying |
|
Thanks @navDhammu good points. I will update to restrict just to Thanks for the conversation about static. That might be a good idea! But not sure it would cover all cases or not. Thanks @kathyavini for the options for |
|
@Duncan-Brain I noticed while reviewing that locally I'm not able run the backend server after building with tsc because its using Something minor - maybe consider using |
|
I love the idea of aligning the folders will check that out! -- I mentioned this difference in my notes section of this PR and I hate that it is different haha I like the compatibility idea! But I can't spend any more time on this now. |
Fix templates syntax Co-authored-by: Joyce Yuki <82857964+kathyavini@users.noreply.github.com>
Description
This PR copies all files from src that are not .cjs, .js, and .ts to the dist folder since non-javascript are not compiled as part of tsc.
This hopefully fixes the no-emails issue where the files are not found. Not 100% sure on this but maybe 90% sure this is it.
Notes:
rsyncwas added to dockerfile as an easy to understand utility, the find and copy was unreadable comparably and it seems like this is more standard for this now?npm build) because the copy path would be different./dist/app/src/vs `./dist/api/src/ so it will work locally with a tweak to that path.Jira link: LF-4842
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Ran the docker container locally by adding this to my docker compose:
Checklist:
pnpm i18nto help with this)