Adding fingerprint to compiled css #694

gedeagas · 2018-05-23T17:43:20Z

Motivation

As stated here #439 fingerprinting the CSS file is necessary when the website use CDN. It is also necessary to fingerprint our static file because in PWAs they cache the content via paths.

Have you read the Contributing Guidelines on pull requests?

Yes

docusaurus-bot · 2018-05-23T17:45:28Z

Deploy preview for docusaurus-preview ready!

Built with commit 59525aa

https://deploy-preview-694--docusaurus-preview.netlify.com

gedeagas · 2018-05-23T18:02:23Z

This is still a WIP.

I am not sure if adding a hash.txt file on the build folder is the right and cleanest approach in order to supply the CSS hash string to the test suites

…Redirect

Modifying build-file test case to read hashed CSS

yangshun

@gedeagas Thanks for the PR! However, I don't agree with this approach that a random hash is generated on every build. I think the better approach is to generate the hash based on file contents instead so that if the file contents doesn't change, the hash remains the same and loaded assets can be cached.

yangshun · 2018-05-25T03:49:08Z

lib/__tests__/build-files.test.js

@@ -88,10 +88,14 @@ test('Generated table of contents', function() {
 });

 test('Concatenated CSS files', function() {
+  let hash = fs.readFileSync(
+    buildDir + '/' + siteConfig.projectName + '/hash.txt',


What is there a need for hash.txt?

the hash.txt file is needed in order to supply the CSS hash string to the test suites. but this is only a temporary fix because as I say in my previous comment adding a hash.txt file on the build folder is not the right nor cleanest approach to this. Do you have any suggestion on this?

yangshun · 2018-05-25T03:50:22Z

lib/core/Head.js

+          <link
+            rel="stylesheet"
+            href={
+              this.props.config.baseUrl + 'css/' + this.props.hash + '.main.css'


Isn't it more common to have main.[hash].css? At least that's the webpack convention.

yangshun · 2018-05-25T03:50:35Z

lib/server/generate.js


  commander.option('--skip-image-compression').parse(process.argv);

+  // create secure crypto hash for css
+  var secureHash = crypto.randomBytes(10).toString('hex');


Use const

gedeagas · 2018-05-25T04:08:41Z

Will try to implement hashing based on the content later today. Thank you for your suggestion @yangshun

…nvention of the ccs file into webpack like convention.

gedeagas · 2018-05-26T16:54:24Z

How do you think @yangshun ?

endiliey · 2018-05-31T09:42:45Z

lib/core/Head.js

-          rel="stylesheet"
-          href={this.props.config.baseUrl + 'css/main.css'}
-        />
+        {this.props.hash ? (


Seems like we can save few lines of code by removing double re-use of <link ....

Example:

href = {`${this.props.config.baseUrl}css/main${this.props.hash ? this.props.hash : ''}.css`}

or ( i prefer this )

href = {this.props.config.baseUrl + 'css/main.' + this.props.hash ? this.props.hash : '' + '.css'}

woah, this not crossed my mind before @endiliey. i think your second option is what we should do

Actually @yangshun told me template literal string is easier to read

this.props.hash ? `${this.props.config.baseUrl}css/main${this.props.hash}.css` : `${this.props.config.baseUrl}css/main.css` :

JoelMarcey · 2018-06-06T20:05:11Z

@gedeagas Hi! I think we are just waiting for the suggested change and a rebase since there are some conflicts now.

endiliey

Please make the suggested changes (refactor), resolve the merge conflicts & provide a test plan whereby the same build(no changes in file content) produces the same hash and different build produces different hash

gedeagas · 2018-06-13T17:33:04Z

Hi @JoelMarcey, I will be working on this today. Sorry for the delay. crazy week at the office.

Got it @endiliey

endiliey

I patched & refactored @gedeagas PR. It should be working now. I tested with 5 different build, and the same build(no changes in file content) produces the same hash and different build produces different hash

Test Plan

No change on main.css
Result: main.1ac8d6325f8699c1b86a3b61854ad4e82f880891.css
No change on main.css
Result: main.1ac8d6325f8699c1b86a3b61854ad4e82f880891.css
Change main.css
Result: main.0a037522d71b06ecf631742f19a4acd6fbd04ce6.css

footer .social {
  padding: 15px 0;
}

Change main.css
Result: main.d06b7994781743bfed253162cc0a59c3c7e0452d.css

footer .social {
  padding: 25px 0;
}

Change main.css back to original
Result: main.1ac8d6325f8699c1b86a3b61854ad4e82f880891.css

Our css is now fingerprinted 😄

Edit review

yangshun · 2018-06-18T17:20:28Z

One thing we need to test is whether the separate css works. For example, Prettier has a separate playground that hardcodes the CSS path there. Will their code break when we introduce this?

endiliey · 2018-06-18T17:27:54Z

By right it should work because the implementation only fingerprint the compiled css from Docusaurus main.css and all css in website/static/css

const secureHash = fingerprintCss(join(CWD, 'static', 'css', '*.css'));

Example:
Our prism.css is defined separately so it's not even hashed & used separately

But just to be safe, I'll test on prettier tmrw

endiliey · 2018-06-19T11:00:21Z

@yangshun

Just tried prettier website, serving the build with python http server. Seems that separate css work as per normal

Only compiled css (compiled to main.css) get hashed

Edit: On a side note, let's not rush to merge this 😄

yangshun · 2018-06-19T15:45:27Z

Another thought - if the playground page referenced main.css (without hash) it would fail right? If not, then I think we can ship this.

…

On Tue, Jun 19, 2018 at 4:00 AM Endilie Yacop Sucipto < ***@***.***> wrote: @yangshun <https://github.com/yangshun> Just tried prettier website. Seems that separate css work as per normal [image: separate css] <https://user-images.githubusercontent.com/17883920/41593505-df384e26-73f2-11e8-81eb-21072966dcd3.PNG> Only main.css get hashed [image: maincss] <https://user-images.githubusercontent.com/17883920/41593531-f4cc5f02-73f2-11e8-9570-5942d498fe3d.PNG> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#694 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQRHVjkak2FQ5rNK1OtHUu_c8jUCQTlks5t-NnIgaJpZM4UK5Y-> .

endiliey · 2018-06-19T16:32:43Z

Yes, if the playground page referenced main.css (without hash). It would fail.

Without this PR

build/prettier/
├── css
│   └── main.css

With this PR it will be something like

build/prettier/
├── css
│   └── main.0cfe7d770fe94c50109893d6354ad47842c8c1fb.css

It's a given that static html page like this didn't know the hash and won't be able to reference main.css.

All other pages (docs, blogs, *.js) won't have a problem because we inject the main.css (with hash) in Head.js

However, a way for static HTML pages to use our own Docusaurus site styles, header and footer is to set wrapPagesHTML to true (https://docusaurus.io/docs/en/site-config#optional-fields)
https://github.com/facebook/Docusaurus/blob/169a5e6ded1d742feabf13878d559dd80fd19384/lib/server/generate.js#L564-L576
Then this one will be injected with the main.css (with hash)

JoelMarcey · 2018-06-19T16:36:37Z

So if we landed this, then Prettier would have to update their site or the playground would break? Just making sure I understand that correctly.

endiliey · 2018-06-19T16:40:44Z

@JoelMarcey it won't break prettier's website because they never reference Docusaurus main.css on their static HTML page. I think we are talking about corner cases where people reference Docusaurus css files directly in .html instead of using proper wrapPagesHTML

endiliey · 2018-06-19T16:46:03Z

I just checked https://docusaurus.io/docs/en/custom-pages#adding-static-pages

Static .html files can also be used, but they will not include Docusaurus' header, footer, or styles by default. These can be added to the static directory in the same way as other static assets. Alternatively, they can be placed in the pages directory and would be served as-is instead of being rendered from React.
If you wish to use Docusaurus' stylesheet, you can access it at ${baseUrl}css/main.css.
You can set the $wrapPagesHTML site config option in order to wrap raw HTML fragments with the Docusaurus site styling, header and footer.

Seems that this will break someone's else code 😂
A very simple solution is that we also generate main.css (without hash) for backward compatibilities

build/prettier/
├── css
│   └── main.0cfe7d770fe94c50109893d6354ad47842c8c1fb.css
│   └── main.css

yangshun · 2018-06-19T16:51:49Z

A very simple solution is that we also generate main.css (without hash) for backward compatibilities

I thought of that too. But doesn't seem to be an elegant solution. If this feature is not that important shall we hold off for now?

endiliey · 2018-06-19T16:55:20Z

I agree, it defeats the purpose of fingerprinting. I think this feature is not that important (sorry @gedeagas 😢). It might become important if we focus on building offline version of Docusaurus. Let's put this on hold

parano · 2018-07-30T23:10:34Z

For those who deploy to CDN and still looking for a workaround, we currently use the following lines in our deployment script to add fingerprint to CSS, before pushing to S3:

GIT_ROOT=$(git rev-parse --show-toplevel)
GIT_HASH=$(git rev-parse HEAD)

cd $GIT_ROOT/website/build/
mv css/main.css css/main-$GIT_HASH.css
find ./ -name '*.html' -print0 | xargs -0 sed -i '' 's/\"\/css\/main.css\"/\"\/css\/main-'$GIT_HASH'.css\"/g'

hope it's helpful

endiliey · 2018-08-22T08:33:34Z

Closing this in favor of v2 implementation

Adding fingerprint to compiled css

dfdf418

facebook-github-bot added the CLA Signed Signed Facebook CLA label May 23, 2018

gedeagas mentioned this pull request May 23, 2018

Add fingerprint to compiled css/js files #439

Closed

Agastya Darma Lakasna and others added 5 commits May 24, 2018 01:14

Adding css hash into BlogPageLayout, BlogPostLayout, DocsLayout, and …

f656505

…Redirect

Disable css hasing on development

ee12add

Modifying build-file test case to read hashed CSS

1dd1bdc

Merge pull request #1 from gedeagas/experimental

4de3e21

Modifying build-file test case to read hashed CSS

Merge branch 'master' into master

cfe713a

yangshun suggested changes May 25, 2018

View reviewed changes

Agastya Darma Lakasna and others added 4 commits May 26, 2018 23:46

Changing the hash based on the CSS content and changing the naming co…

f40933d

…nvention of the ccs file into webpack like convention.

Fixing build-file test to read from webpack style css file naming

b1dd9a2

Merge branch 'master' of https://github.com/gedeagas/Docusaurus

76b285f

Merge branch 'master' into master

f0edbf4

gedeagas mentioned this pull request May 26, 2018

Feature: referencing variables from markdown pages (placeholder substitution) #395

Closed

endiliey reviewed May 31, 2018

View reviewed changes

endiliey added ❌No Test Plan labels Jun 2, 2018

endiliey suggested changes Jun 10, 2018

View reviewed changes

Merge remote-tracking branch 'upstream/master'

0d9f027

endiliey force-pushed the master branch from dddaeaf to 0d9f027 Compare June 18, 2018 15:50

refactor implementation

2d4fd68

endiliey force-pushed the master branch 2 times, most recently from dbb6441 to a5c13f2 Compare June 18, 2018 16:45

refactor fingerprintcss into a function

339b761

endiliey force-pushed the master branch from a5c13f2 to 339b761 Compare June 18, 2018 16:47

endiliey previously approved these changes Jun 18, 2018

View reviewed changes

endiliey added ✅ Test Plan and removed ❌ No Test Plan labels Jun 18, 2018

fix wrong href :)

59525aa

endiliey approved these changes Jun 18, 2018

View reviewed changes

endiliey added status: don't merge yet This PR is completed, but we are still waiting for other things to be ready. do-not-reap proposal This issue is a proposal, usually non-trivial change and removed Major Change labels Jun 19, 2018

endiliey closed this Aug 22, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding fingerprint to compiled css #694

Adding fingerprint to compiled css #694

gedeagas commented May 23, 2018

docusaurus-bot commented May 23, 2018 •

edited

Loading

gedeagas commented May 23, 2018 •

edited

Loading

yangshun left a comment

yangshun May 25, 2018

gedeagas May 25, 2018

gedeagas May 25, 2018

yangshun May 25, 2018

gedeagas May 25, 2018

yangshun May 25, 2018

gedeagas May 25, 2018

gedeagas commented May 25, 2018

gedeagas commented May 26, 2018

endiliey May 31, 2018

gedeagas Jun 5, 2018

endiliey Jun 5, 2018 •

edited

Loading

JoelMarcey commented Jun 6, 2018

endiliey left a comment

gedeagas commented Jun 13, 2018

endiliey left a comment •

edited

Loading

yangshun commented Jun 18, 2018 •

edited

Loading

endiliey commented Jun 18, 2018 •

edited

Loading

endiliey commented Jun 19, 2018 •

edited

Loading

yangshun commented Jun 19, 2018 via email

endiliey commented Jun 19, 2018

JoelMarcey commented Jun 19, 2018

endiliey commented Jun 19, 2018

endiliey commented Jun 19, 2018 •

edited

Loading

yangshun commented Jun 19, 2018

endiliey commented Jun 19, 2018 •

edited

Loading

parano commented Jul 30, 2018

endiliey commented Aug 22, 2018

Adding fingerprint to compiled css #694

Adding fingerprint to compiled css #694

Conversation

gedeagas commented May 23, 2018

Motivation

Have you read the Contributing Guidelines on pull requests?

docusaurus-bot commented May 23, 2018 • edited Loading

gedeagas commented May 23, 2018 • edited Loading

yangshun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gedeagas commented May 25, 2018

gedeagas commented May 26, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

endiliey Jun 5, 2018 • edited Loading

Choose a reason for hiding this comment

JoelMarcey commented Jun 6, 2018

endiliey left a comment

Choose a reason for hiding this comment

gedeagas commented Jun 13, 2018

endiliey left a comment • edited Loading

Choose a reason for hiding this comment

yangshun commented Jun 18, 2018 • edited Loading

endiliey commented Jun 18, 2018 • edited Loading

endiliey commented Jun 19, 2018 • edited Loading

yangshun commented Jun 19, 2018 via email

endiliey commented Jun 19, 2018

JoelMarcey commented Jun 19, 2018

endiliey commented Jun 19, 2018

endiliey commented Jun 19, 2018 • edited Loading

yangshun commented Jun 19, 2018

endiliey commented Jun 19, 2018 • edited Loading

parano commented Jul 30, 2018

endiliey commented Aug 22, 2018

docusaurus-bot commented May 23, 2018 •

edited

Loading

gedeagas commented May 23, 2018 •

edited

Loading

endiliey Jun 5, 2018 •

edited

Loading

endiliey left a comment •

edited

Loading

yangshun commented Jun 18, 2018 •

edited

Loading

endiliey commented Jun 18, 2018 •

edited

Loading

endiliey commented Jun 19, 2018 •

edited

Loading

endiliey commented Jun 19, 2018 •

edited

Loading

endiliey commented Jun 19, 2018 •

edited

Loading