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

chore/upgrade greenwood v0.30.0 #98

Merged
merged 26 commits into from
Nov 2, 2024
Merged

Conversation

thescientist13
Copy link
Contributor

@thescientist13 thescientist13 commented Mar 10, 2024

Related Issue

N / A

Summary of Changes

  1. Upgrade Greenwood v0.30.0
  2. Refactor to use Constructable / Adopted Stylesheets

TODO

  1. CSS url optimization breaking on unresolvable assets - chore/upgrade greenwood v0.30.0 #98 (comment)
  2. test without needing PostCSS import-plugin, instead using ConstructableStylesheets (nice to have) - chore/upgrade greenwood v0.30.0 #98
  3. restore testing workflow
  4. convert everything to ConstructableStylesheets and drop raw plugin (would we still need postcss-import plugin?)
  5. PostCSS nesting not happening? - lol, of course the PostCSS plugin was commented out 😅
    Parse error: Colon is expected
    1 |.as-route-contact {
    2 |
    3 |  p {  <----
    4 |    width: 60%;
    5 |    margin: 0 auto;
  6. netlify deploy preview not inlining CSS? - lol, forgot to setup patch-package postinstall 🤦‍♂️
  7. inline optimization "breaks" prod Constructable StyleSheets and thus causes a 404? (and not stripping <link> tags either?) - I think it's just cause the inline will not cause an emit? So maybe need to check when bundle mapping the assets optimization status and look it up if so?
  8. 🚨 Rollup placeholders intermittently showing up in build output - chore/upgrade greenwood v0.30.0 #98 (comment)
  9. 🚨 Import Attributes not actually supported 🫨 - polyfill configurations for import maps and attributes ProjectEvergreen/greenwood#1268
  10. ga plugin not working now in local serve (race condition?) - seems fine
  11. 🚨 Netlify API redirects are failing all of a sudden? - chore/upgrade greenwood v0.30.0 #98 (comment)
  12. address TS warnings (good first issue?)
    Screenshot 2024-05-30 at 9 33 37 PM
  13. test without needing FA plugin and copying? (nice to have)

Upstreams

  1. better error handling for CSS url optimization and better handling for data: - bug/issue 1199 gracefully handle unresolvable asset URLs during CSS optimization ProjectEvergreen/greenwood#1235
  2. SPA dev routing is being too greedy - bug/issue 1234 refine spa local dev routing for html requests ProjectEvergreen/greenwood#1251
  3. real production constructable stylesheets -enhancement/issue 923 real production import attributes ProjectEvergreen/greenwood#1259
  4. postcss-import / development CSS bundling - feature/issue 1257 CSS optimization workflow parity ProjectEvergreen/greenwood#1258

Copy link

netlify bot commented May 6, 2024

Deploy Preview for practical-fermat-fa2c48 failed.

Name Link
🔨 Latest commit 13cd0f1
🔍 Latest deploy log https://app.netlify.com/sites/practical-fermat-fa2c48/deploys/66db349341f5c500086a5ea3

@thescientist13
Copy link
Contributor Author

Noticed that when combined with the PostCSS plugin-import plugin, FA was being inlined into the final CSS bundles (which is expected because that is its job, hah!)

  body: '/*!\n' +
    ' *  Font Awesome 4.6.3 by @davegandy - http://fontawesome.io - @fontawesome\n' +
    ' *  License - http://fontawesome.io/license (Font: SIL OFL 1.1, CSS: MIT License)\n' +
    ' */\n' +
    '/* FONT PATH\n' +
    ' * -------------------------- */\n' +
    '@font-face {\n' +
    "  font-family: 'FontAwesome';\n" +
    "  src: url('../fonts/fontawesome-webfont.eot?v=4.6.3');\n" +
    "  src: url('../fonts/fontawesome-webfont.eot?#iefix&v=4.6.3') format('embedded-opentype'), url('../fonts/fontawesome-webfont.woff2?v=4.6.3') format('woff2'), url('../fonts/fontawesome-webfont.woff?v=4.6.3') format('woff'), url('../fonts/fontawesome-webfont.ttf?v=4.6.3') format('truetype'), url('../fonts/fontawesome-webfont.svg?v=4.6.3#fontawesomeregular') format('svg');\n" +
    '  font-weight: normal;\n' +
    '  font-style: normal;\n' +
    '}\n' +
    '.fa {\n' +

It means Greenwood's CSS optimization has no reference for where to start looking and because of no node_modules signal in the pathname, fails to look it up and completely breaks on lookup, which is obviously not good.

  Error: ENOENT: no such file or directory, open '/Users/owenbuckley/Workspace/analogstudiosri/www.analogstudios.net/src/fonts/fontawesome-webfont.eot'
    at Object.openSync (node:fs:596:3)
    at Object.readFileSync (node:fs:464:35)
    at Object.enter (file:///Users/owenbuckley/Workspace/analogstudiosri/www.analogstudios.net/node_modules/@greenwood/cli/src/plugins/resource/plugin-standard-css.js:56:36)
    at walkNode (file:///Users/owenbuckley/Workspace/analogstudiosri/www.analogstudios.net/node_modules/css-tree/lib/walker/create.js:169:36)
    at List.walkReducer (file:///Users/owenbuckley/Workspace/analogstudiosri/www.analogstudios.net/node_modules/css-tree/lib/walker/create.js:195:61)
    at List.reduce (file:///Users/owenbuckley/Workspace/analogstudiosri/www.analogstudios.net/node_modules/css-tree/lib/utils/List.js:174:22)
    at Object.Value (file:///Users/owenbuckley/Workspace/analogstudiosri/www.analogstudios.net/node_modules/css-tree/lib/walker/create.js:103:31)
    at walkNode (file:///Users/owenbuckley/Workspace/analogstudiosri/www.analogstudios.net/node_modules/css-tree/lib/walker/create.js:180:41)
    at Object.Declaration (file:///Users/owenbuckley/Workspace/analogstudiosri/www.analogstudios.net/node_modules/css-tree/lib/walker/create.js:108:28)
    at walkNode (file:///Users/owenbuckley/Workspace/analogstudiosri/www.analogstudios.net/node_modules/css-tree/lib/walker/create.js:180:41) {
  errno: -2,
  syscall: 'open',
  code: 'ENOENT',
  path: '/Users/owenbuckley/Workspace/analogstudiosri/www.analogstudios.net/src/fonts/fontawesome-webfont.eot'

Should at least guard for this with a warning message, which I have patched in for now and will need to upstream

if (fs.existsSync(locationUrl.pathname)) {
  // do all the fancy stuff
  optimizedCss += `url('${basePath}${hashedRoot}')`;         
} else {
  // TODO handle  'data:', e.g.  Unable to optimize /Users/owenbuckley/Workspace/analogstudiosri/www.analogstudios.net/src/data:image/svg+xml;charset=utf8,%3Csvg%20xmlns='http://www.w3.org/2000/svg'%20viewBox='0%200%204%204'%3E%3Cpath%20stroke='%23fff'%20d='M0%202h4'/%3E%3C/svg%3E.
  console.warn(`Unable to optimize ${locationUrl.pathname}.  You may need to manually copy this file into the output directory.`);
  optimizedCss += `url('${value}')`;
}

@thescientist13
Copy link
Contributor Author

thescientist13 commented May 31, 2024

Wow, this is really nice now using ConstructableStylesheets, and removing the inline @import reference in the CSS is paying off huge for payload sizes 🤩

BEFORE

Before, a copy of Bootstrap and FA would be part of every component bundle file

@import "../../theme.css";

.as-header {
  /** CSS goes here **/
}
import { css, html, LitElement, unsafeCSS, TemplateResult } from 'lit';
import { customElement } from 'lit/decorators.js';
import '../navigation/navigation.ts';
import headerCss from './header.css?type=css';

@customElement('app-header')
export class HeaderComponent extends LitElement {
  static styles = css`${unsafeCSS(headerCss)}`;

  protected render(): TemplateResult {
    return html`
      <header class="as-header">
        <div id="as-inner-header">
          <h1 class="as-header__logo"><a title="Home Page" href="/">Analog Studios</a></h1>
          <app-navigation></app-navigation>
        </div>
      </header>
    `;
  }
}

Screenshot 2024-05-30 at 9 52 08 PM

And thus would really bloat the bundle sizes
production-bundle-payload-sizes

AFTER

Now, we can join the two together

.as-header {
  /** CSS goes here **/
}
import { css, html, LitElement, unsafeCSS, TemplateResult } from 'lit';
import { customElement } from 'lit/decorators.js';
import '../navigation/navigation.ts';
import headerCss from './header.css?type=css';

@customElement('app-header')
export class HeaderComponent extends LitElement {
  static styles = css`${unsafeCSS(headerCss)}`;

  protected render(): TemplateResult {
    return html`
      <header class="as-header">
        <div id="as-inner-header">
          <h1 class="as-header__logo"><a title="Home Page" href="/">Analog Studios</a></h1>
          <app-navigation></app-navigation>
        </div>
      </header>
    `;
  }
}

And now the site payload has dropped by 500KB !
Screenshot 2024-05-30 at 9 59 44 PM


In fact, we could probably make all the components use Constructable Styleheets too!

import { html, LitElement, TemplateResult } from 'lit';
import { customElement } from 'lit/decorators.js';
import '../navigation/navigation.ts';
import theme from '../../theme.css' with { type: 'css' };
import header from './header.css' with { type: 'css' };

@customElement('app-header')
export class HeaderComponent extends LitElement {
  static styles = [theme, header];

  protected render(): TemplateResult {
    return html`
      <!-- HTML goes here -->
    `;
  }
}

@thescientist13 thescientist13 added feature New feature or request and removed needs upstream labels Jun 29, 2024
@thescientist13
Copy link
Contributor Author

thescientist13 commented Aug 3, 2024

Looks like there might a race condition happening somewhere related to greenwoodSyncImportAttributes in rollup.config.js, and seems specifically related to the use of optimization: 'inline' in greenwood.config.js, in that some of the built files are still including placeholders for Rollup asset bundling, e.g.

import o from"/import.meta.ROLLUP_ASSET_URL_2926c8e7"with{type:"css"};
Screenshot 2024-08-03 at 6 20 07 PM Screenshot 2024-08-03 at 6 18 42 PM Screenshot 2024-08-03 at 6 19 44 PM

My guess is that since we are doing these replacements in the generateBundle Rollup hook, the Greenwood code that runs the inline optimization on the HTML code is happening before Rollup has finished running. 🫢


Hmm, so no after upgrading and pulling in changes for ProjectEvergreen/greenwood#1268 I can't seem to reproduce this issue anymore after restoring the inline optimization setting. soooo I guess we're good for now? 🥺

@thescientist13 thescientist13 marked this pull request as ready for review November 2, 2024 17:18
@thescientist13
Copy link
Contributor Author

thescientist13 commented Nov 2, 2024

Sooooo... now Netlify redirects seems to be failing now? 😳
Screenshot 2024-11-02 at 1 20 49 PM

Local proxy was fine, and so is production?
https://www.analogstudios.net/api/v2/posts


and it works 🤷‍♂️
Screenshot 2024-11-02 at 7 18 11 PM

@thescientist13 thescientist13 merged commit fac8efd into main Nov 2, 2024
5 checks passed
@thescientist13 thescientist13 deleted the chore/upgrade-greenwood-0.30.0 branch November 2, 2024 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant