Skip to content

build-sass: Create output directory if missing#8384

Merged
aduth merged 6 commits intomainfrom
aduth-build-sass-mkdir
May 18, 2023
Merged

build-sass: Create output directory if missing#8384
aduth merged 6 commits intomainfrom
aduth-build-sass-mkdir

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented May 11, 2023

🛠 Summary of changes

Enhances @18f/identity-build-sass to create the output directory if it does not already exist, to avoid imposing a requirement that the folder exists already. This avoids the need for workarounds like app/assets/builds/.keep.

Part of the goal of this is also to start establishing a pattern for testing CLI behaviors in the package.

📜 Testing Plan

yarn build:css should pass still, even after removing app/assets/builds/.keep.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note:

  • We deleted app/assets/builds/.keep (an empty file)
  • We added an empty file for the fixture, in.css.scss

Git interpreted this as a "move".

@aduth aduth force-pushed the aduth-build-sass-mkdir branch from 0b9ecdd to 9e24f74 Compare May 12, 2023 13:05
@aduth
Copy link
Contributor Author

aduth commented May 12, 2023

We might need the .keep file after all, not because of build-sass, but because of the asset pipeline. 😕

     Failure/Error: //= link_tree ../builds
     
     ActionView::Template::Error:
       link_tree argument must be a directory

Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might need the .keep file after all, not because of build-sass, but because of the asset pipeline. 😕

     Failure/Error: //= link_tree ../builds
     
     ActionView::Template::Error:
       link_tree argument must be a directory

How does that play with the propshaft PR?

the .gitkeep isn't the end of the world

either way, LGTM

@aduth
Copy link
Contributor Author

aduth commented May 15, 2023

How does that play with the propshaft PR?

the .gitkeep isn't the end of the world

either way, LGTM

Not sure, actually! Good question though. I can test it.

aduth added 4 commits May 16, 2023 08:54
changelog: Internal, Build Tooling, Create stylesheet output directory if missing
This reverts commit 9e24f7402cdb6e732adfbf07fa1908673ecb58a6.
@aduth aduth force-pushed the aduth-build-sass-mkdir branch from 455c10f to d844407 Compare May 16, 2023 12:55
@aduth
Copy link
Contributor Author

aduth commented May 16, 2023

Rebased after merge of #8387 and re-removed the .keep file in d844407, let's see how the build tolerates it.

@aduth
Copy link
Contributor Author

aduth commented May 16, 2023

Apparently still no dice 😢 Might need to keep it for now at least.

The error is a little different. Previously Sprockets complained about link_tree needing to reference a directory. Now it's just claiming that "'application.css' was not found in the load path". The file should exist at that point in app/assets/builds, which makes me wonder if it might be checking for the existence of the directory before the CSS finishes building.

@aduth
Copy link
Contributor Author

aduth commented May 16, 2023

Apparently it's "required" per the cssbundling-rails documentation:

A common issue is that your repository does not contain the output directory used by the build commands. You must have app/assets/builds available. Add the directory with a .gitkeep file, and you'll ensure it's available in production.

https://github.com/rails/cssbundling-rails#why-do-i-get-applicationcss-not-in-asset-pipeline-in-production

@aduth
Copy link
Contributor Author

aduth commented May 17, 2023

I might plan to publish this as a release prior to #8375, which includes a breaking change for the package.

(Just means bumping the version and updating the CHANGELOG accordingly)

@aduth
Copy link
Contributor Author

aduth commented May 18, 2023

Published to NPM as @18f/identity-build-sass@1.3.0.

@aduth aduth merged commit f5ab890 into main May 18, 2023
@aduth aduth deleted the aduth-build-sass-mkdir branch May 18, 2023 12:53
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.

2 participants