Uds 1958 #1515
Uds 1958 #1515
Conversation
scott-williams-az
commented
Apr 10, 2025
scott-williams-az
commented
- JIRA ticket
|
Storybook deployed at https://unity-uds-staging.s3.us-west-2.amazonaws.com/pr-1515/index.html |
davidornelas11
left a comment
davidornelas11
left a comment
There was a problem hiding this comment.
Looks good. Just a few things I noticed
| resolve(__dirname, "src/js/global-header.js"), | ||
| resolve(__dirname, "src/js/data-layer.js"), | ||
| resolve(__dirname, "../../node_modules/bootstrap/js/index.esm.js"), | ||
| // resolve(__dirname, "../../node_modules/bootstrap/js/index.esm.js"), |
There was a problem hiding this comment.
This can just be removed since we just copy the bootstrap umd over now.
| if (chunkInfo.name.includes("index.esm")) { | ||
| return "js/bootstrap.bundle.min.[format]"; | ||
| return "js/[format]/bootstrap.bundle.min.js"; | ||
| } |
There was a problem hiding this comment.
This can also be removed now since the copy-bootstrap-umd-to-dist plugin up top already moves the bootstrap umd over to the dist file. This just overwrites that anyways
| "module": "./src/js/storybook-data-layer.js", | ||
| "default": "./src/js/data-layer.js" | ||
| }, | ||
| "./*": "./*", |
There was a problem hiding this comment.
We just have to make sure we update our README to provide any info needed for imprting certain files
| */ | ||
| cardLink: PropTypes.string, | ||
| }; | ||
|
|
There was a problem hiding this comment.
Nice job removing the default props on these. I think there is a ticket for this so we might be able to close that too. I will check
|
|
||
| import pkg from "./package.json"; | ||
| /** @typedef {import('vite').UserConfig} UserConfig */ | ||
| import baseConfig from "./vite.config.js"; |
There was a problem hiding this comment.
Good idea reusing the build config
| plugins: [ | ||
| { | ||
| name: "copy", | ||
| writeBundle() { |
There was a problem hiding this comment.
One small caveat here is that this runs 3 times, one for each format. The closeBundle hook only runs once after the entire build process runs. This is not an issue in local but in the CI that might be underpowered and doing this dozens of times, it might add up. I console logged the time it takes to process and its a difference of 500-800ms for closeBundle vs 2-4 seconds for writeBundle. Might be different on Jenkins. Screenshot is only how many times it runs
There was a problem hiding this comment.
If we were doing anything with the build itself then we would have to call bundle.close() but we should be fine not needing to call it
