jaegertracing / documentation

Documentation/website for the Jaeger Distributed Tracing project.
https://jaegertracing.io/
Apache License 2.0
68 stars 164 forks source link

Help upgrading bulma #730

Open yurishkuro opened 3 months ago

yurishkuro commented 3 months ago

The bot is trying to upgrade bulma modules in the SASS setup:

diff --git a/themes/jaeger-docs/package.json b/themes/jaeger-docs/package.json
index b9f67da..67195ff 100644
--- a/themes/jaeger-docs/package.json
+++ b/themes/jaeger-docs/package.json
@@ -1,7 +1,7 @@
 {
   "private": true,
   "devDependencies": {
-    "bulma": "^0.7.1",
-    "bulma-tooltip": "^2.0.2"
+    "bulma": "^1.0.0",
+    "bulma-tooltip": "^3.0.0"
   }
 }

If after the above change I go to cd themes/jaeger-docs/ and run npm install to update the vendored dependencies, then the build later fails with SASS errors:

$ make develop
...
ERROR TOCSS: failed to transform "/sass/style.sass" (text/x-sass): "/Users/ysh/dev/jaegertracing/documentation/themes/jaeger-docs/node_modules/bulma/sass/utilities/functions.scss:34:28": Invalid CSS after "...olor-base: list": expected expression (e.g. 1px, bold), was ".nth($components, 1"
Built in 9512 ms
Error: error building site: TOCSS: failed to transform "/sass/style.sass" (text/x-sass): "/Users/ysh/dev/jaegertracing/documentation/themes/jaeger-docs/node_modules/bulma/sass/utilities/functions.scss:34:28": Invalid CSS after "...olor-base: list": expected expression (e.g. 1px, bold), was ".nth($components, 1"
make: *** [develop] Error 1
vsoroten commented 3 months ago

Hello, I'm new to the jaeger project but I'd like to start contributing. I would like to look at this-- I've started by looking over changes that occurred in the bulma project between versions 0.7.1 and 1.0.0.

yurishkuro commented 3 months ago

@vsoroten we welcome contributions. As a suggestion, going through all changes across versions of these modules might be a lot of work, it may be easier to start with the actual error message and investigate what's causing it.

vsoroten commented 3 months ago

@yurishkuro I was able to reproduce the error. I was also able to update up to bulma v0.9.4 without error. The jump from bulma v0.9.4 to bulma v1.0.0 seems to be a common issue https://github.com/jgthms/bulma/issues/3867. Looking into upgrading into bulma v1.0.0 now.

vsoroten commented 3 months ago

Hey @yurishkuro an update --- I have gotten the project to build under bulma v1.0.0 and it seems like the initial issue was because bulma v1.0.0 now uses DartSass instead of sass. The issue I'm seeing now, is that the projects builds but no css is being applied.

So far I've tried using the sass-migrator tool for sass modules under themes/jaeger-docs to perform the conversion to DartSass with no success.

So I took a step back and dove deeper into the migration guide for bulma which didn't offer too many hints. With bulma v1.0.0 I'm able to get the website to render if I naively change all of the @import rules in styles.sass to @use rules, but the website renders as though no css has been applied, which suggests I've broken a link somewhere between bulma's .scss files and the project -- I'm not sure where though. Do you have any advice going forward? I'm still very eager to solve this albeit slowly since this is my first exposure to bulma, sass, hugo and the like.

yurishkuro commented 3 months ago

I don't have any concrete suggestions, this isn't my area of expertise. Is it possible that Hugo directly supports sass without using Node modules? This build process was implemented quite a while ago.

vsoroten commented 3 months ago

Hey @yurishkuro, not sure what else to explore with tackling the issue. So far I've been able to get the website to build with Bulma v1.0.0. by using the sass-migrator tool with a process like...

cd themes/jaeger-docs/assets/sass/
sass-migrator --load-path ../../node_modules module style.sass

which converted the Sass to DartSass automatically for all .sass files in this directory.

However, when rendering the website, it seems that CSS still isn't applied.

Is it possible that Hugo directly supports sass without using Node modules?

It is possible for Hugo to directly support sass. But are you suggesting that we remove the dependency on Bulma in package.json and just install it via

@import "https://cdn.jsdelivr.net/npm/bulma@1.0.0/css/bulma.min.css";

instead? I also see that the project did not originally include Bulma in Node modules this issue https://github.com/jaegertracing/documentation/pull/139 gave me more context for why this was chosen.

For now I'm not sure how to grok the issue further since it seems like when there aren't any errors, there is a silent failure (like with the CSS not being rendered).

yurishkuro commented 3 months ago

Looking at the links you provided, dart sass option in Hugo is just another transpiler, I don't really see much difference on how it makes the integration easier. So I think it would make more sense to stick with bulma and figure out why it doesn't work.

EpicGuy4000 commented 2 months ago

So, I've taken a look at the issue and here are my finding @yurishkuro, which align with what @vsoroten said. In 1.0.0 bulma switches to using Dart Sass (as per documentation) Among the new sass modules which bulma started using is sass:list module, which as per documentation which is only compatible with Dart Sass (since version 1.23.0). Hugo extended includes LibSass instead (as per documentation), which is not compatible with sass:list.

I've added a PR #733 with non visual changed. I'm not exactly an expert on visuals, so if someone else can update them it would be great, otherwise I may work on it, but it may take a while.