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

Migrate from SASS to SCSS #247

Closed
hravnx opened this issue Apr 14, 2019 · 16 comments
Closed

Migrate from SASS to SCSS #247

hravnx opened this issue Apr 14, 2019 · 16 comments

Comments

@hravnx
Copy link
Contributor

hravnx commented Apr 14, 2019

If I add a style to the (initially empty) style.sass file, it will fail to compile.

To reproduce:

  1. Create an empty folder and navigate into it
  2. Run dotnet new SAFE -lang F#
  3. Run fake build --target run and wait for the browser to come up and present the counter example.
  4. Open the src/Client/style.sass file in the editor of your choice and add a style to it. For example:
h3 {
  width: 100px;
}
  1. Save it. Observe the error in the compilation process:
 ERROR in ./src/Client/style.sass (./node_modules/css-loader/dist/cjs.js!./node_modules/sass-loader/lib/loader.js??ref--6-2!./src/Client/style.sass)
Module build failed (from ./node_modules/sass-loader/lib/loader.js):

h3 {
  ^
     Expected newline.
h3 {
   ^
      stdin 1:4  root stylesheet
      in /Users/username/Projects/Spikes/sss/src/Client/style.sass (line 1, column 4)
 @ ./src/Client/style.sass 2:14-136 21:1-42:3 22:19-141
 @ multi (webpack)-dev-server/client?http://localhost:8080 (webpack)/hot/dev-server.js ./src/Client/style.sass

OS: macOS 10.14.4
SAFE template: 1.0.1
dotnet core: 2.2.202

@Zaid-Ajaj
Copy link
Contributor

Hmm no sass expert here but the docs say the syntax is a bit different:

h3
  width: 100px

@hravnx
Copy link
Contributor Author

hravnx commented Apr 14, 2019

You are right, of course - I normally use scss syntax. Silly of me, thanks :-)

@hravnx hravnx closed this as completed Apr 14, 2019
@Zaid-Ajaj
Copy link
Contributor

No problem! I think the template also includes a CSS loader, so you can create a plain old .css file and import it like this in your entry file on the Client project:

import "./custom-styles.css" 

of course assuming custom-styles.css is a sibling file to entry file that contains the import statement

@hravnx
Copy link
Contributor Author

hravnx commented Apr 14, 2019

I just renamed style.sassto style.scss and changed the cssEntry property in webpack.config.js and happiness ensued. Thanks again for the quick help!

@theimowski
Copy link
Member

I choose the SASS over SCSS as I saw it was being used somewhere in the sample Fable projects. @alfonsogarciacaro @MangelMaxime do you have an opinion on whether any is preferred nowadays?

@alfonsogarciacaro
Copy link
Contributor

Apparently, SCSS is the new syntax and is designed to be a superset of CSS3 so I guess it makes sense to use it from now on: https://stackoverflow.com/a/5654471

@MangelMaxime
Copy link
Contributor

You can use SASS or SCSS syntax. This is a superset of CSS3 like mentioned by @alfonsogarciacaro

Originally, I was using SASS syntax as done in Bulma but because SASS syntax is no longer evolving (SCSS is the new version of SASS and is the one maintained). There is still some few pains points when using it that will never be fixed...

For example, if you use a dict you need to use it as a single line instruction:

$colors: mergeColorMaps(("white": ($white, $black), "black": ($black, $white), "light": ($light, $light-invert), "dark": ($dark, $dark-invert), "primary": ($primary, $primary-invert), "link": ($link, $link-invert), "info": ($info, $info-invert), "success": ($success, $success-invert), "warning": ($warning, $warning-invert), "danger": ($danger, $danger-invert)), $custom-colors) !default

In the mean time, if you use SCSS you can write something like that:

$marker-colors : (
    "red": ($marker-red, $marker-red-invert),
    "turquoise": ($marker-turquoise, $marker-turquoise-invert),
    "green-pale": ($marker-green-pale, $marker-green-pale-invert),
    "yellow": ($marker-yellow, $marker-yellow-invert),
    "brown": ($marker-brown, $marker-brown-invert),
    "blue": ($marker-blue, $marker-blue-invert),
    "green-dark": ($marker-green-dark, $marker-green-dark-invert),
    "orange": ($marker-orange, $marker-orange-invert),
    "grey-metallic": ($marker-grey-metallic, $marker-grey-metallic-invert),
    "purple": ($marker-purple, $marker-purple-invert),
    "blue-dark": ($marker-blue-dark, $marker-blue-dark-invert),
    "green": ($marker-green, $marker-green-invert),
    "brown-light": ($marker-brown-light, $marker-brown-light-invert),
    "pink": ($marker-pink, $marker-pink-invert),
    "grey-light": ($marker-grey-light, $marker-grey-light-invert),
);

So I am in favor of using SCSS but both can work side by side. This is actually the case in most of my projects, I have CSS, SASS and SCSS working side by side :)

@theimowski
Copy link
Member

Thanks for your input.

I love it how this comment with largest number of upvotes summarises the CSS development:

When choosing the syntax, keep in mind that only scss allows copying and pasting css from stackoverflow and the browsers' development tools, whereas in sass you always have to adjust the syntax

Given above feedback I think we should change the default in template from SASS to SCSS, unless someone has strong opinion why we shouldn't do that.

@theimowski theimowski reopened this Apr 16, 2019
@theimowski theimowski changed the title Adding styles to style.sass fails to compile Migrate from SASS to SCSS Apr 16, 2019
@hravnx
Copy link
Contributor Author

hravnx commented Apr 16, 2019

Ok, I can take a stab at this, since I'm the reason this happened :-)

What should I do with the version number? 1.0.2? 1.1.0? 2.0.0?

@theimowski
Copy link
Member

Awesome! Don't worry about version number, I'll bump it after PR is accepted

@hravnx
Copy link
Contributor Author

hravnx commented Apr 16, 2019

Ok, I think I'm almost there. However, running fake build to test it locally modifies the Paket.Restore.targets file. That seems counter-intuitive.

Should I just ignore that, or commit the changed file, or?

@theimowski
Copy link
Member

Please ignore Paket.Restore.targets file. TBF I'm not sure how this file gets updated as well

@hravnx
Copy link
Contributor Author

hravnx commented Apr 16, 2019

PR created here

@MangelMaxime
Copy link
Contributor

@theimowski I am almost certain that Paket.Restore.targets is being updated by paket.

I think that something there is a change in the files to fix it or support new things when paket is being upgraded. You can fix the version of paket using version XXX in paket.dependencies. And actually, you should have a warning coming from paket in the console asking you to do so. (TBF I never fix the paket version unless I discover a bug but that's kind of a bad "idea" because the build is not reproducible without locking the version just like with the packages).

@theimowski
Copy link
Member

@MangelMaxime yeah probably pinning version XXX in paket.dependencies shouldn't touch the targets file, however I think I once encountered issue where the file changed even though I had the version pinned. I'll report when I can reproduce that.

I do agree though that the targets file changing from time to time might be misleading for most users

@theimowski
Copy link
Member

Closed by #248

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

5 participants