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

Bundling! 🤘 #36

Merged
merged 2 commits into from
Mar 30, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Bundling! 🤘
  • Loading branch information
matthewp committed Mar 29, 2021
commit eae58cd5cb72890c810622066f315ad33999cddb
1 change: 1 addition & 0 deletions examples/snowpack/astro.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ export default {
projectRoot: '.',
astroRoot: './astro',
dist: './_site',
public: './public',
extensions: {
'.jsx': 'preact',
},
Expand Down
1 change: 1 addition & 0 deletions examples/snowpack/astro/pages/guides.astro
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ let communityGuides;

<div class="card-grid card-grid-4">
{communityGuides.map((post) => <Card item={post} />)}

<Card item={{
url: 'https://www.snowpack.dev/posts/2021-01-13-snowpack-3-0',
img: 'https://www.snowpack.dev/img/social-snowpackv3.jpg',
Expand Down
6 changes: 3 additions & 3 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
"postcss-modules": "^4.0.0",
"react": "^17.0.1",
"react-dom": "^17.0.1",
"rollup": "^2.43.1",
"sass": "^1.32.8",
"snowpack": "^3.1.2",
"svelte": "^3.35.0",
Expand Down
2 changes: 2 additions & 0 deletions src/@types/astro.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ export interface AstroConfigRaw {
dist: string;
projectRoot: string;
astroRoot: string;
public?: string;
jsx?: string;
}

Expand All @@ -11,6 +12,7 @@ export interface AstroConfig {
dist: string;
projectRoot: URL;
astroRoot: URL;
public?: URL;
extensions?: Record<string, ValidExtensionPlugins>;
}

Expand Down
98 changes: 71 additions & 27 deletions src/build.ts
Original file line number Diff line number Diff line change
@@ -1,34 +1,65 @@
import type { AstroConfig } from './@types/astro';
import { defaultLogOptions, LogOptions } from './logger';
import type { LogOptions } from './logger';
import type { LoadResult } from './runtime';

import { loadConfiguration, startServer as startSnowpackServer, build as snowpackBuild } from 'snowpack';
import { promises as fsPromises } from 'fs';
import { relative as pathRelative } from 'path';
import { defaultLogDestination, error } from './logger.js';
import { createRuntime } from './runtime.js';
import { bundle, collectDynamicImports } from './build/bundle.js';
import { collectStatics } from './build/static.js';

const { mkdir, readdir, stat, writeFile } = fsPromises;
const { mkdir, readdir, readFile, stat, writeFile } = fsPromises;

const logging: LogOptions = {
level: 'debug',
dest: defaultLogDestination,
};

async function* allPages(root: URL): AsyncGenerator<URL, void, unknown> {
async function* recurseFiles(root: URL): AsyncGenerator<URL, void, unknown> {
for (const filename of await readdir(root)) {
const fullpath = new URL(filename, root);
const info = await stat(fullpath);

if (info.isDirectory()) {
yield* allPages(new URL(fullpath + '/'));
yield* recurseFiles(new URL(fullpath + '/'));
matthewp marked this conversation as resolved.
Show resolved Hide resolved
} else {
if (/\.(astro|md)$/.test(fullpath.pathname)) {
yield fullpath;
}
yield fullpath;
}
}
}

async function* allPages(root: URL): AsyncGenerator<URL, void, unknown> {
for await(const filename of recurseFiles(root)) {
if(/\.(astro|md)$/.test(filename.pathname)) {
yield filename;
}
}
}

function mergeSet(a: Set<string>, b: Set<string>) {
for(let str of b) {
a.add(str);
}
return a;
}

async function writeFilep(outPath: URL, bytes: string | Buffer, encoding: 'utf-8' | null) {
const outFolder = new URL('./', outPath);
await mkdir(outFolder, { recursive: true });
await writeFile(outPath, bytes, encoding || 'binary');
}

async function writeResult(result: LoadResult, outPath: URL, encoding: null | 'utf-8') {
if(result.statusCode !== 200) {
error(logging, 'build', result.error || result.statusCode);
//return 1;
} else {
const bytes = result.contents;
await writeFilep(outPath, bytes, encoding);
}
}

export async function build(astroConfig: AstroConfig): Promise<0 | 1> {
const { projectRoot, astroRoot } = astroConfig;
const pageRoot = new URL('./pages/', astroRoot);
Expand All @@ -39,39 +70,52 @@ export async function build(astroConfig: AstroConfig): Promise<0 | 1> {
dest: defaultLogDestination,
};

const runtime = await createRuntime(astroConfig, { logging: runtimeLogging, env: 'build' });
const { snowpackConfig } = runtime.runtimeConfig;

try {
const result = await snowpackBuild({
config: snowpackConfig,
lockfile: null,
});
} catch (err) {
error(logging, 'build', err);
return 1;
}
const runtime = await createRuntime(astroConfig, { logging: runtimeLogging });
const { runtimeConfig } = runtime;
const { snowpack } = runtimeConfig;
const resolve = (pkgName: string) => snowpack.getUrlForPackage(pkgName)

const imports = new Set<string>();
const statics = new Set<string>();

for await (const filepath of allPages(pageRoot)) {
const rel = pathRelative(astroRoot.pathname + '/pages', filepath.pathname); // pages/index.astro
const pagePath = `/${rel.replace(/\.(astro|md)/, '')}`;

try {
const outPath = new URL('./' + rel.replace(/\.(astro|md)/, '.html'), dist);
const outFolder = new URL('./', outPath);
const result = await runtime.load(pagePath);

if (result.statusCode !== 200) {
error(logging, 'generate', result.error || result.statusCode);
//return 1;
} else {
await mkdir(outFolder, { recursive: true });
await writeFile(outPath, result.contents, 'utf-8');
await writeResult(result, outPath, 'utf-8');
if(result.statusCode === 200) {
mergeSet(statics, collectStatics(result.contents.toString('utf-8')));
}
} catch (err) {
error(logging, 'generate', err);
return 1;
}

mergeSet(imports, await collectDynamicImports(filepath, astroConfig, resolve));
}

await bundle(imports, {dist, runtime, astroConfig});

for(let url of statics) {
const outPath = new URL('.' + url, dist);
const result = await runtime.load(url);

await writeResult(result, outPath, null);
}

if(astroConfig.public) {
const pub = astroConfig.public;
for await(const filename of recurseFiles(pub)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think overall the correct behavior is to simply copy files from the /public directory as-is, as you‘ve done here. But I wanted to highlight that .scss files end up in the final build in /public when we don‘t need them.

But this begs the question: if we ignore Sass, what else do we ignore? I think it‘s better behavior to not try and treeshake a user’s assets (e.g. they may just want to have download links or serve files publicly). But that said, what if we ignored Sass files, and only Sass files for now? There‘s not really ever a scenario where these need to be public, IMO.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to just be very explicit about zero building going on in the public/ directory, and every file getting copied over 1:1. If you put a Sass file in the public directory, you must want it hosted for some reason. Otherwise, move them into astro (ex: you can create your own astro/styles folder for external styles that need to be built)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main reason that this is copying over public is that dynamic components mean you can't statically analyze the pages to determine what assets are used.

Agree on treating public files as just bytes.

const rel = pathRelative(pub.pathname, filename.pathname);
const outUrl = new URL('./' + rel, dist);

const bytes = await readFile(filename);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just out of curiosity, if we wanted to do some optimization like image optimization in the future, how hard would it be to provide a hook into this? Or is that something to just worry about later?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this process is very much an MVP one and I'm not sure what a final version will look like. This is doing some stuff that presumably we'd prefer Snowpack do, but that would take a bit of work to refactor snowpack to allow. So extensibility was not really thought about at all at this time. So if you need to add something, just add whereever.

await writeFilep(outUrl, bytes, null);
}
}

await runtime.shutdown();
Expand Down
Loading