Build/bundle assets and CSS #1786

matthewp · 2021-11-10T16:37:06Z

Changes

This updates the build so that assets and CSS are also included.
This is achieved by creating two new rollup plugins: one handles HTML scanning and updating, and the other handles building CSS.

Testing

CSS Bundling test back in.
Asset test back in.

Docs

N/A

changeset-bot · 2021-11-10T16:37:09Z

🦋 Changeset detected

Latest commit: a1c7c48

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
astro	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

packages/astro/src/core/build/index.ts

jasikpark · 2021-11-10T18:28:32Z

🤔 btw when trying to build www on this branch (cb1e874) I get this output:

Is there a way I can get better introspection into what's going on?

jasikpark · 2021-11-10T19:02:55Z

     ✔ is only called once during build

READING /D:/a/astro/astro/packages/astro/test/fixtures/astro-global/src/images/penguin.png
READING /D:/a/astro/astro/packages/astro/test/fixtures/astro-global/src/components/nested/images/penguin.png
[@astro/rollup-plugin-build] ENOENT: no such file or directory, open 'D:\D:\a\astro\astro\packages\astro\test\fixtures\astro-global\src\images\penguin.png'

(node:4508) PromiseRejectionHandledWarning: Promise rejection was handled asynchronously (rejection id: 5)
  Astro.*
    2) "before all" hook for "Astro.request.url"

that's pretty weird 🤔 (https://github.com/snowpackjs/astro/runs/4169598680?check_suite_focus=true#step:7:236)

matthewp · 2021-11-10T19:21:07Z

@jasikpark looks like www needs to be updated before it can be built, it is using some imports that do not work with Vite.

jasikpark · 2021-11-10T19:31:22Z

@jasikpark looks like www needs to be updated before it can be built, it is using some imports that do not work with Vite.

right, when testing on this branch + the changes in #1775 though, I still get:

Maybe that's not an assets thing though 🤷‍♂️

matthewp · 2021-11-10T20:01:32Z

@jasikpark I just rebased with main and don't see that. Maybe try pulling this branch again, running yarn, then yarn build and letting me know.

drwpow · 2021-11-10T20:06:03Z

packages/astro/package.json

@@ -66,7 +66,7 @@
    "@babel/traverse": "^7.15.4",
    "@proload/core": "^0.2.1",
    "@proload/plugin-tsm": "^0.1.0",
-    "@web/rollup-plugin-html": "^1.10.1",


jasikpark · 2021-11-10T20:13:37Z

@jasikpark I just rebased with main and don't see that. Maybe try pulling this branch again, running yarn, then yarn build and letting me know.

I still have the same problem... maybe it's somehow just a problem in Gitpod??? Want to enable the www smoketest in ci.yml` and see if that changes anything?

drwpow · 2021-11-10T20:07:46Z

packages/astro/src/core/util.ts

@@ -71,3 +71,7 @@ export function resolveDependency(dep: string, astroConfig: AstroConfig) {
  // For Windows compat, we need a fully resolved `file://` URL string
  return pathToFileURL(resolved).toString();
 }
+
+export function viteifyPath(pathname: string): string {


Ha I like the name

drwpow · 2021-11-10T20:09:42Z

packages/astro/src/core/build/types.d.ts

@@ -0,0 +1,10 @@
+import type { ComponentPreload } from '../ssr/index';
+import type { RouteData } from '../../@types/astro-core';


What’s the reason for this type file living here and not @types?

drwpow · 2021-11-10T20:10:01Z

packages/astro/src/core/build/index.ts

-          input,
-          extractAssets: false,
-        }) as any, // "any" needed for CI; also we don’t need typedefs for this anyway
+        rollupPluginAstroBuildHTML({


drwpow · 2021-11-10T20:11:34Z

packages/astro/src/core/ssr/index.ts

+    const [renderers, mod] = await preload(ssrOpts);
+    return render(renderers, mod, ssrOpts);    
+  } catch (e: unknown) {
+    await errorHandler(e, ssrOpts.viteServer, ssrOpts.filePath);


Nice cleanup!

packages/astro/src/vite-plugin-build-css/index.ts

drwpow · 2021-11-10T20:14:47Z

packages/astro/src/vite-plugin-build-css/index.ts

+
+const ASTRO_PAGE_STYLE_PREFIX = '@astro-page-all-styles';
+
+const isCSSRequest = (request: string) => STYLE_EXTENSIONS.has(path.extname(request));


Vite may sometimes append query params to URLs, e.g. styles.css?direct. Probably safest to make sure those are being handled as well.

drwpow · 2021-11-10T20:15:37Z

packages/astro/src/@types/astro-runtime.ts

@@ -48,6 +48,7 @@ export interface TopLevelAstro {

 export interface SSRMetadata {
  renderers: Renderer[];
+  pathname: string;


Where is pathname used? Can this be a URL or no?

drwpow · 2021-11-10T20:16:08Z

packages/astro/src/core/build/index.ts

+          allPages[route.component] = {
+            route,
+            paths: [route.pathname],
+            preload: await ssrPreload({


I like the preload approach. Clever

drwpow · 2021-11-10T20:22:33Z

packages/astro/src/vite-plugin-build-html/index.ts

+          });
+          renderedPageMap.set(id, html);
+
+          const document = parse5.parse(html, {


Just a sidenote: if we like parse5 better we should probably replace htmlparser2 with this (I didn’t profile parse5; I just went with htmlparser2 because it’s fast, easy-to-use, and actively maintained)

Was going to be my comment as well. I'm fine with either, but we should only use one.

natemoo-re

Looking good! Drew covered all of my comments ☺️

matthewp · 2021-11-10T20:47:54Z

packages/astro/src/vite-plugin-astro/styles.ts

-  if (!PREPROCESSOR_EXTENSIONS.has(lang)) {
+export async function transformWithVite({ value, attrs, transformHook, id, ssr, force }: TransformWithViteOptions): Promise<vite.TransformResult | null> {
+  let lang = (`.${attrs.lang}` || '').toLowerCase(); // add leading "."; don’t be case-sensitive
+  if (!force && !PREPROCESSOR_EXTENSIONS.has(lang)) {


note that this is not a needed change any more

Co-authored-by: Drew Powers <1369770+drwpow@users.noreply.github.com>

hans-d · 2021-11-11T21:43:55Z

Not fully working, eg https://stackblitz.com/edit/github-d4rp6r?file=package.json - npm run dev works, npm run build not.
https://discord.com/channels/830184174198718474/908470416736149564

* Bundling CSS * Current progress of building assets * New build progress * Its finally working * Force css to go through the build * Prettier filenames * Split into separate CSS and HTML plugins * Always have at least one input * Bring back in sitemaps + output * Bring back srcset support * Bundle CSS * Bring back minify * Update dynamic tests * Update remaining tests * Linting * Fix remaining broken test * Use fs directly * Adding a changeset * Use path.posix * Debugging windows * More debugging * Pass URLs into readFile * Remove some debugging stuff * Remove force flag from transformWithVite * Update packages/astro/src/vite-plugin-build-css/index.ts Co-authored-by: Drew Powers <1369770+drwpow@users.noreply.github.com> Co-authored-by: Drew Powers <1369770+drwpow@users.noreply.github.com>

matthewp requested a review from a team as a code owner November 10, 2021 16:37

matthewp commented Nov 10, 2021

View reviewed changes

packages/astro/src/core/build/index.ts Outdated Show resolved Hide resolved

matthewp force-pushed the build-assets branch from 78277a1 to d7a5898 Compare November 10, 2021 16:42

jasikpark mentioned this pull request Nov 10, 2021

🐛 BUG: Assets in src/ do not get build in astro@next #1715

Closed

jasikpark linked an issue Nov 10, 2021 that may be closed by this pull request

🐛 BUG: Assets in src/ do not get build in astro@next #1715

Closed

natemoo-re added the 0.21 label Nov 10, 2021

natemoo-re added this to the v0.21 milestone Nov 10, 2021

matthewp added 18 commits November 10, 2021 15:00

Bundling CSS

4299d94

Current progress of building assets

320e69c

New build progress

a328eb8

Its finally working

b9ad35b

Force css to go through the build

dd11dee

Prettier filenames

ddb16ec

Split into separate CSS and HTML plugins

4b8ba0a

Always have at least one input

7198f30

Bring back in sitemaps + output

5cd3e6b

Bring back srcset support

93ceac8

Bundle CSS

bedc135

Bring back minify

90ce9e7

Update dynamic tests

60fed7c

Update remaining tests

a93e3da

Linting

c81b16b

Fix remaining broken test

1f260a2

Use fs directly

3553177

Adding a changeset

9f2ea74

matthewp added 5 commits November 10, 2021 15:00

Use path.posix

b467f1f

Debugging windows

9693f7f

More debugging

4f4b21c

Pass URLs into readFile

7124a57

Remove some debugging stuff

757ca1a

matthewp force-pushed the build-assets branch from 01135ad to 757ca1a Compare November 10, 2021 20:00

drwpow reviewed Nov 10, 2021

View reviewed changes

natemoo-re reviewed Nov 10, 2021

View reviewed changes

matthewp commented Nov 10, 2021

View reviewed changes

drwpow approved these changes Nov 10, 2021

View reviewed changes

matthewp and others added 2 commits November 10, 2021 16:11

Remove force flag from transformWithVite

005a488

Update packages/astro/src/vite-plugin-build-css/index.ts

a1c7c48

Co-authored-by: Drew Powers <1369770+drwpow@users.noreply.github.com>

matthewp merged commit fd52bce into main Nov 11, 2021

matthewp deleted the build-assets branch November 11, 2021 13:44

schlichtanders mentioned this pull request Jan 24, 2022

🐛 BUG: Build/bundle assets not working #2453

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build/bundle assets and CSS #1786

Build/bundle assets and CSS #1786

matthewp commented Nov 10, 2021

changeset-bot bot commented Nov 10, 2021 •

edited

jasikpark commented Nov 10, 2021 •

edited

jasikpark commented Nov 10, 2021

matthewp commented Nov 10, 2021

jasikpark commented Nov 10, 2021 •

edited

matthewp commented Nov 10, 2021

drwpow Nov 10, 2021

jasikpark commented Nov 10, 2021

drwpow Nov 10, 2021

drwpow Nov 10, 2021

drwpow Nov 10, 2021

drwpow Nov 10, 2021

drwpow Nov 10, 2021

drwpow Nov 10, 2021

drwpow Nov 10, 2021

drwpow Nov 10, 2021

natemoo-re Nov 10, 2021

natemoo-re left a comment

matthewp Nov 10, 2021

hans-d commented Nov 11, 2021

		@@ -0,0 +1,10 @@
		import type { ComponentPreload } from '../ssr/index';
		import type { RouteData } from '../../@types/astro-core';


		const ASTRO_PAGE_STYLE_PREFIX = '@astro-page-all-styles';

		const isCSSRequest = (request: string) => STYLE_EXTENSIONS.has(path.extname(request));

Build/bundle assets and CSS #1786

Build/bundle assets and CSS #1786

Conversation

matthewp commented Nov 10, 2021

Changes

Testing

Docs

changeset-bot bot commented Nov 10, 2021 • edited

🦋 Changeset detected

jasikpark commented Nov 10, 2021 • edited

jasikpark commented Nov 10, 2021

matthewp commented Nov 10, 2021

jasikpark commented Nov 10, 2021 • edited

matthewp commented Nov 10, 2021

Choose a reason for hiding this comment

jasikpark commented Nov 10, 2021

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

Choose a reason for hiding this comment

natemoo-re left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hans-d commented Nov 11, 2021

changeset-bot bot commented Nov 10, 2021 •

edited

jasikpark commented Nov 10, 2021 •

edited

jasikpark commented Nov 10, 2021 •

edited