administrate
administrate copied to clipboard
thoughtbot
Compile assets with dart-sass and esbuild
Exciting! Is there a way to lint whether assets have been re-compiled?
Exciting! Is there a way to lint whether assets have been re-compiled?
Not sure how to do that... but maybe I can trigger the assets build each time rake is called?
@n-studio - Thank you for this. Just dropping a quick line to let you know that this is being looked into.
The first thing that I have noticed is that the generated assets break my test environment. For example: if I run the specs locally (Rails 6.1 at the moment), line 46 below breaks because the CSS is not loaded.
https://github.com/thoughtbot/administrate/blob/be688bc39b76984d49c7ba220f9ed8c86f2187ba/spec/features/edit_page_spec.rb#L39-L48
As far as I can tell, this is because the generated assets gain precedence over the ones that Rails 6.1 would load, but there's some issue loading them and the browser only gets a 404, thus not getting a stylesheet.
I haven't had time to dig deeper than that for now. If you or other people interested in the PR could look into it, that would be great.
@pablobm When I run the specs with Rails 6.1 locally all my specs pass green. Maybe you haven't compiled the assets before running the tests? If you run rake spec assets won't be compiled before running the tests, you have to either run rake or rake compile_assets; rake spec. Or did I miss something?
@pablobm The date picker is broken indeed, let me fix it. Btw can you help me silence Hound? I'm not familiar with it and I couldn't find how to exclude generated assets from being linted.
I fixed the date picker and rebased here: https://github.com/n-studio/administrate/pull/2 I will merge in this branch when I figure out how to silence hound.
Does someone know how to silence
hound? This is annoying...
Add an exclude to .scss-lint.yml for the compiled files.
@danielmorrison I added:
scss_files: "lib/assets/stylesheets/**/*.scss"
exclude:
- "lib/assets/stylesheets/reset/**"
- "app/assets/builds/**/*"
but it didn't work...
I think I figured out the issue with Hound. I merged a fix (hopefully) at https://github.com/thoughtbot/administrate/pull/2138. Try rebasing and let's see if it works?
Forgot to mention: after rebasing, you'll have to add an ignoreFiles entry to .stylelintrc.json. See https://stylelint.io/user-guide/configure/#ignorefiles
@pablobm The Hound fix didn't seem to work 😖
To fix date-time-picker I had to copy the css from https://github.com/c4lliope/datetime_picker_rails
I couldn't figure out where the css was in npm repositories, my guess is that the css in the gem has been customized.
If https://github.com/thoughtbot/administrate/pull/2136 is accepted I can also drop date-time-picker completely.
On the contrary, the change worked perfectly :slightly_smiling_face: The warnings about that generated CSS file are now gone, and what we have left is different and worth looking into.
- Some shoud be trivial, like the line length ones.
- The
spaced-commentone is from eslint. I would add an exception to that specific file - The ones about
importbeing a reserved word... I think it's eslint too, and it can be configured to understand it: https://programmerah.com/solved-parsing-error-the-keyword-import-is-reserved-35034/ - The
function-calc-no-invalidones look like it's misinterpreting the hyphens in the variable name as arithmetic "minus" signs. Looking up the issue, it looks like it might be possible to fix it: https://github.com/stylelint-scss/stylelint-scss/blob/master/src/rules/dollar-variable-pattern/README.md - Etc.
Would you be able to sort those?
@pablobm Yes, sorry I've been busy, I try to fix that asap. What do you think about https://github.com/thoughtbot/administrate/pull/2136 ?
@n-studio - No rush. Re: #2136, there's a question open on compatibility, but I think we'll resolve it soon. Perhaps do as if it's happening for now, and don't worry too much about the date picker.
Thank you for opening this and the effort you've taken to keep it up to date so far. I've been thinking about this PR over the last few days and I'm thinking that there's a completely different approach to take: remove Sass entirely.
This PR makes quite a lot of changes in one go, which I'm not so keen on doing. Introducing dart-sass like this needs us to change how we're handling JavaScript dependencies at the same time. There's been a lot of churn over the last few years in how Rails asset handling has been recommended, and whilst we do need to do a lot of work here, I'm keen to go slowly on it and ride out the change until it settles down.
A long-running issue we've been having is out of date dependencies because they've come from a time when we'd install them from Ruby Gems. It's worthwhile first removing or updating how we do those before making big changes to the way we handle assets, as our potential solution then might be very different. I become aware of dartsass-rails today, too.
For Sass itself, we're not using many of the features which it supports and a quick review suggests that leaning on CSS variables will be good enough to replace those we already use, and hard coding colours where we've calculated the values.
I've roped in @dcyoung-dev to try out the Sass replacement, which I'd like to see the results of (plus us removing other out of date dependencies) before making any decisions here.
Hi @nickcharlton, thank you for your reply!
I'm using my own fork for my own projects so I'm not in a hurry for a new release, but I think other people might be interested in having a release compatible with Rails 7 as soon as possible. I would recommend to use my PR first and then remove or update the dependencies one by one and update the css to avoid sass completely, which seems to be a time consuming task.
I agree, this pr would allow me to use administrate in my project. It could be taken as a temporary solution while you remove Sass.
Closing in favour of https://github.com/thoughtbot/administrate/issues/2311