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

index.css does not match index.css.map #12325

Closed
2 of 7 tasks
CL-Jeremy opened this issue Jul 25, 2020 · 19 comments
Closed
2 of 7 tasks

index.css does not match index.css.map #12325

CL-Jeremy opened this issue Jul 25, 2020 · 19 comments

Comments

@CL-Jeremy
Copy link
Contributor

  • Gitea version (or commit ref): a3afb88
  • Git version: irrelevant but 2.25.4
  • Node version: (I think this is more relevant) 12.16.3
  • Operating system: irrelevant but Fedora Linux 31
  • Database (use [x]):
    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL)
    • No
    • Not relevant
  • Log gist:

Description

This is related to #11997.
It might be my fault to have failed to understand how exactly the build system works, or even be a bug somewhere in the toolchain, but the content of /public/css/index.css does not match /public/css/index.css.map.

This is tested multiple times, even after a complete rebuild using git clean -xdf --exclude=node_modules/* . followed by TAGS="bindata" make build.

In particular, I moved sans-serif to the end of the font stacks in the function .override-fonts, which emerged in d78bb1d and worked fine up till 1.11.x. Going through both files shows that the function does not resolve into what it was before (compare index.css.map or index.css from any version between 1.9.x and 1.11.x by searching for :lang or so).

Screenshots

(It does involve the web interface, but you could probably check https://mikeslab.dix.asia/gitea/CL-Jeremy/gitea/pulls/1).

@silverwind
Copy link
Member

I can only imagine this being a bug in either cssnano-webpack-plugin or cssnano. The former was switched from optimize-css-assets-webpack-plugin specifically because of better source map support.

@CL-Jeremy
Copy link
Contributor Author

I’ve just tested and it appeared already in #9983. It’s not the fact that the source map was any wrong, but rather correct whereas the actual CSS is faulty. Can’t imagine how that could happen since the translator is the same, isn’t it? And if cssnano were to blame, how come it didn’t happen earlier (the commit right before that PR was alright FYI)?

@silverwind
Copy link
Member

Can you try with ui.USE_SERVICE_WORKER = false? Thought highly unlikely it has any effect, I just want to ensure this is not a serviceworker cache issue (I think those are not totally resolved yet).

@CL-Jeremy
Copy link
Contributor Author

CL-Jeremy commented Jul 26, 2020

I wasn’t actually talking about the result, but the actual files generated in the build directory. The resulting static file was already different after #9983. Now the fact a CSS file mismatches the source map is another weird thing.

But if you mean serviceworker would generate something else that shadows index.css, then it becomes even weirder...

Edit: added the setting. Effective rules do not change (as expected). And double-checked the static files are freshly generated each time.
Just tested again, and it’s indeed a bug somewhere in the translation. Swapping @default-fonts against any other expressions (e.g. Test, @monospaced-fonts, 'A B C') in the :lang blocks would resolve correctly.

Edit 2: something very curious happened when I do make clean-all && make generate: the mixins seem read from some cache and unifies the results, giving (at global scope) something like:

.markdown:not(code),textarea{font-family:BlinkMacSystemFont,system-ui,-apple-system,Segoe UI,Roboto,Ubuntu,Cantarell,Noto Sans,sans-serif,Helvetica,Arial}h1,h2,h3,h4,h5{font-family:Roboto,BlinkMacSystemFont,system-ui,-apple-system,Segoe UI,Ubuntu,Cantarell,Noto Sans,sans-serif,Helvetica,Arial}.home .hero h1,.home .hero h2{font-family:PT Sans Narrow,Roboto,BlinkMacSystemFont,system-ui,-apple-system,Segoe UI,Ubuntu,Cantarell,Noto Sans,sans-serif,Helvetica,Arial}

with

- @default-fonts: -apple-system, BlinkMacSystemFont, system-ui, 'Segoe UI', Roboto, Helvetica, Arial;
+ @default-fonts: BlinkMacSystemFont, system-ui, 'Segoe UI', Roboto, Helvetica, Arial;

NB: this is all tested with detached HEAD at 1019913ea.

@CL-Jeremy
Copy link
Contributor Author

I did a quick grep -rl -- '-apple-system' . and the result (after removing it from _base.less) was:

./node_modules/swagger-ui/dist/swagger-ui.css.map
./node_modules/swagger-ui/dist/swagger-ui.css
./node_modules/stylelint/lib/rules/font-family-no-duplicate-names/README.md
./node_modules/stylelint/lib/rules/font-family-name-quotes/README.md
./node_modules/postcss-preset-env/README.md
./node_modules/postcss-preset-env/index.mjs.map
./node_modules/postcss-preset-env/index.mjs
./node_modules/postcss-preset-env/index.js.map
./node_modules/postcss-preset-env/index.js
./node_modules/less/test/css/3rd-party/bootstrap4.css
./node_modules/csso/dist/csso.min.js
./node_modules/csso/dist/csso.js
./node_modules/css-tree/data/patch.json
./node_modules/css-tree/dist/default-syntax.json
./node_modules/css-tree/dist/csstree.min.js
./node_modules/css-tree/dist/csstree.js
./public/vendor/plugins/vue-calendar-heatmap/vue-calendar-heatmap.css

Any chance they could do anything strange here? I believe none of the above actually comes from #9983.

@silverwind
Copy link
Member

But if you mean serviceworker would generate something else that shadows index.css

No, that's not possible. I was just thinking maybe it was serving a cached asset before your change but I'm pretty sure cache invalidation is working correctly for it and .map files are probable not even cached by the worker.

@silverwind
Copy link
Member

I think it's more likely you've hit some bug in the Less source map generation. I want to move off Less variables to CSS variables for the fonts (#11045) which may fix that situation.

@silverwind
Copy link
Member

Is this still an issue? We've now switched to the new css-minimizer-webpack-plugin and I personally have not noticed any issue with its sourcemaps.

@CL-Jeremy
Copy link
Contributor Author

I'll take a look. Running this on my own web server takes almost forever (screenshot from the version referred to in this issue, just fired up), but I don't want to test it locally...
Bildschirmfoto 2020-08-17 um 21 10 37

@silverwind
Copy link
Member

200s template emission time? Something is definitely wrong there, nevery seen anything like it.

@CL-Jeremy
Copy link
Contributor Author

200s template emission time? Something is definitely wrong there, nevery seen anything like it.

No, 28.4s (but still remarkable). This is mainly due to some SSD configuration of my VPS I believe. CPU is constantly at 100% upon starting gitea for around 3 minutes...

@silverwind
Copy link
Member

silverwind commented Aug 17, 2020

Certainly strange. Maybe related to #7910. If the VPS still has enough resources left, maybe some of the golang debugging tools like pprof could help debug these issues.

@CL-Jeremy
Copy link
Contributor Author

Yeah, compared to Gogs on the same server, Gitea is at least 50% slower. I'm limiting the resources using cgroups, though. Have seen too many occasions where things get OOM-killed, and that single-core CPU is very precious for the handful of services I keep there (JVM and Node.js are both famous resource hoggers).

@CL-Jeremy
Copy link
Contributor Author

No dice. Still the same thing. As long as this mixin is used as it is right now, it would always go in that way.

I still suspect the problem happens in webpack less compiler itself somehow. I don't know the lifecycle, though.

@silverwind
Copy link
Member

Can this be reproduced on https://try.gitea.io? For example if I view the font definitions in Chrome:

image

Clicking the line number seems to correctly show the Less source:

image

@CL-Jeremy
Copy link
Contributor Author

You have just proven my point. The definition is:

@default-fonts: -apple-system, BlinkMacSystemFont, system-ui, 'Segoe UI', Roboto, Helvetica, Arial, "Apple Color Emoji", "Segoe UI Emoji", "Segoe UI Symbol", "Noto Color Emoji" sans-serif;
[...]
.override-fonts(@default-fonts);

Yet the result is:

-apple-system,BlinkMacSystemFont,system-ui,Segoe UI,Roboto,Ubuntu,Cantarell,Noto Sans,sans-serif,Helvetica,Arial,Apple Color Emoji,Segoe UI Emoji,Segoe UI Symbol,Noto Color Emoji

Apart from the comma before sans-serif, this still doesn't add up. Where do Ubuntu and Cantarell come from? Certainly not from semantic.

@CL-Jeremy
Copy link
Contributor Author

Damn, I see where it comes from:
Bildschirmfoto 2020-08-18 um 12 24 11

@CL-Jeremy
Copy link
Contributor Author

Not sure if this is idiomatic but it's working now:

diff --git a/webpack.config.js b/webpack.config.js
index d7f0c83d8..8a2f2dbc0 100644
--- a/webpack.config.js
+++ b/webpack.config.js
@@ -4,7 +4,11 @@ const CssMinimizerPlugin = require('css-minimizer-webpack-plugin');
 const FixStyleOnlyEntriesPlugin = require('webpack-fix-style-only-entries');
 const MiniCssExtractPlugin = require('mini-css-extract-plugin');
 const MonacoWebpackPlugin = require('monaco-editor-webpack-plugin');
-const PostCSSPresetEnv = require('postcss-preset-env');
+const PostCSSPresetEnv = () => require('postcss-preset-env')({
+  features: {
+    'system-ui-font-family': false,
+  }
+});
 const TerserPlugin = require('terser-webpack-plugin');
 const VueLoaderPlugin = require('vue-loader/lib/plugin');
 const {statSync} = require('fs');

@silverwind
Copy link
Member

silverwind commented Aug 18, 2020

Ah, I wasn't aware of that postcss-preset-env did that, nice find. Seems resonable we disable that, it's a unnecessary transform.

CL-Jeremy added a commit to CL-Jeremy/gitea that referenced this issue Aug 18, 2020
CL-Jeremy added a commit to CL-Jeremy/gitea that referenced this issue Aug 28, 2020
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants