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

core.css assets are not properly bundled with css-library package as released on NPM #3640

Open
2 of 6 tasks
caseywilliams opened this issue Dec 18, 2024 · 3 comments
Open
2 of 6 tasks
Assignees

Comments

@caseywilliams
Copy link

Bug Report

  • I’ve searched for any related issues and avoided creating a duplicate issue.

What happened

I am a developer on the Okapi team, which maintains the developer-portal. We recently received a dependabot PR to update formation to v12, but the removal of code in v12 breaks most of the styles on our site, so we figured it would be the right time to switch to css-library instead of formation.

The design system website currently mentions that formation is almost deprecated, but it doesn't have instructions on setting up css-library instead. I figured out most of changes required for variable and file names based on closed issues in vets-website, but the core.css file has introduced some more significant issues:

It looks like none of the assets in core.css (fonts or images) can be resolved when importing the css file in a webpack environment (or outside of one, but I would expect that), because the paths to these assets are absolute paths that do not exist in the distributed package on NPM.

After this discussion in DSVA Slack, version css-library 0.16.1-rc1 was shipped to provide the image assets in the bundle. This is partially helpful because I think I'll be able to create some webpack rules that remap the expected location of the images to the correct path in dist. But because the actual url(...) values in the core.css file still refer to locations that don't exist, I still can't load the file in developer-portal by itself.

What I expected to happen

I expected the assets to load without error when I imported the file.

It seems like the natural solution would be to update the url(...) values in core.css so that they use relative paths that are valid within the dist directory, instead of absolute paths from the vets-website/content-build context.

Alternatively, allowing consumers to to define their own css or sass variable to set the base path for assets could help as a workaround. I see that $image-path is used in the original source here, for example. If we could successfully set that $image-path variable before importing css-library, that would be a smoother workaround than configuring webpack to remap the urls as I expect I'll need to do right now.

Reproducing

css-library version: 0.16.1-rc1

I created a small example app here on codesandbox that shows the kinds of errors I'm seeing when importing core.css in my project. To see the behavior, you can comment or uncomment the line in src/index.js file that imports core.css and reload the page (you will need to fork the project first). These errors prevent the project from loading in a browser. The example includes some local url(...) assets as well to show that it is possible for webpack to load them if the paths are valid.

Here's a copy of the text of the first error:

ERROR in ./node_modules/@department-of-veterans-affairs/css-library/dist/stylesheets/core.css (./node_modules/css-loader/dist/cjs.js!./node_modules/@department-of-veterans-affairs/css-library/dist/stylesheets/core.css) 27:37-109
Module not found: Error: Can't resolve '/assets/fonts/merriweather-light-webfont.eot' in '/project/workspace/node_modules/@department-of-veterans-affairs/css-library/dist/stylesheets'

This error originates with this line, which references the font file using a path that does not exist in the published NPM package (/assets/fonts/merriweather-light-webfont.eot?#iefix")

Similar errors happen for every other asset in the core.css file as well.

Urgency

How urgent is this request? Please select the appropriate option below and/or provide details

  • This bug is blocking work currently in progress
  • This bug is affecting work currently in progress but we have a workaround
  • This bug is blocking work planned within the next few sprints
  • This bug is not blocking any work
  • Other

Details

@caw310
Copy link
Contributor

caw310 commented Dec 18, 2024

Please add your estimate of 1, 2, 3, 5, 8 or 13
@Andrew565 - 5
@ataker - 5
@harshil1793 - 5
@it-harrison - 5
@jamigibbs - 5
@micahchiang
@powellkerry - 5
@rmessina1010 - 5
@rsmithadhoc

@jamigibbs
Copy link
Contributor

Related PR that adds the img folder to dist:

@jamigibbs
Copy link
Contributor

jamigibbs commented Dec 19, 2024

Some tasks I think will be required for this issue (probably not complete so don't take this as gospel or even completely correct):

  • Add existing CSS Library package images to the dist output folder which is being done in this PR.
  • Check if the images that were added to vets-website for the merged core swap PR were also added to CSS Library. If some are missing from CSS Library, add them there too.
  • See if there is a way for both vets-website and external projects to operate with the same asset path solution (See the What I expected to happen section in the ticket description).
    • But right now, vets-website does not use the CSS Library assets directly because of the way the build is expecting those images in an S3 bucket (this is a paraphrase and maybe not completely correct). That's why we didn't have images in the CSS library dist folder initially and probably also why we had issues loading fonts in vets-website previously.
  • If vets-website and external projects need to handle assets differently because there's not a straight forward way to update CSS Library to do that, write some documentation on how an external project could use the CSS Library assets (ie. copy the images into the project with webpack)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants