From 7c7ae3dd6d48c3f423a9fe5e7e39c33d76c30435 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 7 Oct 2018 21:47:13 -0400 Subject: [PATCH 1/9] make build/export promise-based --- src/api.ts | 10 ++--- src/api/build.ts | 36 ++++------------ src/api/dev.ts | 3 +- src/api/export.ts | 34 +++++---------- src/api/utils/noop.ts | 1 + src/cli/build.ts | 63 +++++++++++---------------- src/cli/export.ts | 48 +++++++++------------ test/apps/basics/__test__.ts | 41 ++++++------------ test/apps/credentials/__test__.ts | 42 +++++++----------- test/apps/encoding/__test__.ts | 42 +++++++----------- test/apps/errors/__test__.ts | 42 +++++++----------- test/apps/export/__test__.ts | 53 +++++++++-------------- test/apps/ignore/__test__.ts | 42 +++++++----------- test/apps/layout/__test__.ts | 42 +++++++----------- test/apps/preloading/__test__.ts | 42 +++++++----------- test/apps/redirects/__test__.ts | 42 +++++++----------- test/apps/scroll/__test__.ts | 42 +++++++----------- test/apps/store/__test__.ts | 42 +++++++----------- test/apps/with-basepath/__test__.ts | 66 +++++++++++------------------ 19 files changed, 263 insertions(+), 470 deletions(-) create mode 100644 src/api/utils/noop.ts diff --git a/src/api.ts b/src/api.ts index b8cee728e..d90cdfd22 100644 --- a/src/api.ts +++ b/src/api.ts @@ -1,6 +1,4 @@ -import { dev } from './api/dev'; -import { build } from './api/build'; -import { exporter } from './api/export'; -import { find_page } from './api/find_page'; - -export { dev, build, exporter, find_page }; \ No newline at end of file +export { dev } from './api/dev'; +export { build } from './api/build'; +export { export } from './api/export'; +export { find_page } from './api/find_page'; \ No newline at end of file diff --git a/src/api/build.ts b/src/api/build.ts index c9d93f4f1..2dc2041f4 100644 --- a/src/api/build.ts +++ b/src/api/build.ts @@ -2,37 +2,23 @@ import * as fs from 'fs'; import * as path from 'path'; import mkdirp from 'mkdirp'; import rimraf from 'rimraf'; -import { EventEmitter } from 'events'; import minify_html from './utils/minify_html'; import { create_compilers, create_main_manifests, create_manifest_data, create_serviceworker_manifest } from '../core'; -import * as events from './interfaces'; import { copy_shimport } from './utils/copy_shimport'; import { Dirs } from '../interfaces'; import read_template from '../core/read_template'; +import { CompileResult } from '../core/create_compilers/interfaces'; +import { noop } from './utils/noop'; type Opts = { legacy: boolean; bundler: 'rollup' | 'webpack'; + oncompile: ({ type, result }: { type: string, result: CompileResult }) => void; }; -export function build(opts: Opts, dirs: Dirs) { - const emitter = new EventEmitter(); +export async function build(opts: Opts, dirs: Dirs) { + const { oncompile = noop } = opts; - execute(emitter, opts, dirs).then( - () => { - emitter.emit('done', {}); // TODO do we need to pass back any info? - }, - error => { - emitter.emit('error', { - error - }); - } - ); - - return emitter; -} - -async function execute(emitter: EventEmitter, opts: Opts, dirs: Dirs) { rimraf.sync(path.join(dirs.dest, '**/*')); mkdirp.sync(`${dirs.dest}/client`); copy_shimport(dirs.dest); @@ -58,9 +44,8 @@ async function execute(emitter: EventEmitter, opts: Opts, dirs: Dirs) { const { client, server, serviceworker } = await create_compilers(opts.bundler); const client_result = await client.compile(); - emitter.emit('build', { + oncompile({ type: 'client', - // TODO duration/warnings result: client_result }); @@ -72,9 +57,8 @@ async function execute(emitter: EventEmitter, opts: Opts, dirs: Dirs) { const client_result = await client.compile(); - emitter.emit('build', { + oncompile({ type: 'client (legacy)', - // TODO duration/warnings result: client_result }); @@ -86,9 +70,8 @@ async function execute(emitter: EventEmitter, opts: Opts, dirs: Dirs) { fs.writeFileSync(path.join(dirs.dest, 'build.json'), JSON.stringify(build_info)); const server_stats = await server.compile(); - emitter.emit('build', { + oncompile({ type: 'server', - // TODO duration/warnings result: server_stats }); @@ -102,9 +85,8 @@ async function execute(emitter: EventEmitter, opts: Opts, dirs: Dirs) { serviceworker_stats = await serviceworker.compile(); - emitter.emit('build', { + oncompile({ type: 'serviceworker', - // TODO duration/warnings result: serviceworker_stats }); } diff --git a/src/api/dev.ts b/src/api/dev.ts index 37833ed7b..12cd03ed9 100644 --- a/src/api/dev.ts +++ b/src/api/dev.ts @@ -16,6 +16,7 @@ import validate_bundler from '../cli/utils/validate_bundler'; import { copy_shimport } from './utils/copy_shimport'; import { ManifestData } from '../interfaces'; import read_template from '../core/read_template'; +import { noop } from './utils/noop'; export function dev(opts) { return new Watcher(opts); @@ -455,8 +456,6 @@ class DevServer { } } -function noop() {} - function watch_dir( dir: string, filter: ({ path, stats }: { path: string, stats: fs.Stats }) => boolean, diff --git a/src/api/export.ts b/src/api/export.ts index 9b9fa4603..b12f67752 100644 --- a/src/api/export.ts +++ b/src/api/export.ts @@ -4,44 +4,32 @@ import * as sander from 'sander'; import * as url from 'url'; import fetch from 'node-fetch'; import * as ports from 'port-authority'; -import { EventEmitter } from 'events'; import clean_html from './utils/clean_html'; import minify_html from './utils/minify_html'; import Deferred from './utils/Deferred'; -import * as events from './interfaces'; +import { noop } from './utils/noop'; type Opts = { build: string, dest: string, static: string, basepath?: string, - timeout: number | false + timeout: number | false, + oninfo: ({ message }: { message: string }) => void; + onfile: ({ file, size, status }: { file: string, size: number, status: number }) => void; }; -export function exporter(opts: Opts) { - const emitter = new EventEmitter(); - - execute(emitter, opts).then( - () => { - emitter.emit('done', {}); // TODO do we need to pass back any info? - }, - error => { - emitter.emit('error', { - error - }); - } - ); - - return emitter; -} - function resolve(from: string, to: string) { return url.parse(url.resolve(from, to)); } type URL = url.UrlWithStringQuery; -async function execute(emitter: EventEmitter, opts: Opts) { +export { _export as export }; + +async function _export(opts: Opts) { + const { oninfo = noop, onfile = noop } = opts; + const export_dir = path.join(opts.dest, opts.basepath); // Prep output directory @@ -67,7 +55,7 @@ async function execute(emitter: EventEmitter, opts: Opts) { const root = resolve(origin, opts.basepath || ''); if (!root.href.endsWith('/')) root.href += '/'; - emitter.emit('info', { + oninfo({ message: `Crawling ${root.href}` }); @@ -98,7 +86,7 @@ async function execute(emitter: EventEmitter, opts: Opts) { body = minify_html(body); } - emitter.emit('file', { + onfile({ file, size: body.length, status diff --git a/src/api/utils/noop.ts b/src/api/utils/noop.ts new file mode 100644 index 000000000..628cbaede --- /dev/null +++ b/src/api/utils/noop.ts @@ -0,0 +1 @@ +export function noop() {} \ No newline at end of file diff --git a/src/cli/build.ts b/src/cli/build.ts index 20051bba6..a118ca1d5 100644 --- a/src/cli/build.ts +++ b/src/cli/build.ts @@ -4,51 +4,36 @@ import { locations } from '../config'; import validate_bundler from './utils/validate_bundler'; import { repeat } from '../utils'; -export function build(opts: { bundler?: string, legacy?: boolean }) { +export function build(opts: { bundler?: 'rollup' | 'webpack', legacy?: boolean }) { const bundler = validate_bundler(opts.bundler); if (opts.legacy && bundler === 'webpack') { throw new Error(`Legacy builds are not supported for projects using webpack`); } - return new Promise((fulfil, reject) => { - try { - const emitter = _build({ - legacy: opts.legacy, - bundler - }, { - dest: locations.dest(), - src: locations.src(), - routes: locations.routes() - }); - - emitter.on('build', event => { - let banner = `built ${event.type}`; - let c = colors.cyan; - - const { warnings } = event.result; - if (warnings.length > 0) { - banner += ` with ${warnings.length} ${warnings.length === 1 ? 'warning' : 'warnings'}`; - c = colors.yellow; - } - - console.log(); - console.log(c(`┌─${repeat('─', banner.length)}─┐`)); - console.log(c(`│ ${colors.bold(banner) } │`)); - console.log(c(`└─${repeat('─', banner.length)}─┘`)); - - console.log(event.result.print()); - }); - - emitter.on('error', event => { - reject(event.error); - }); - - emitter.on('done', event => { - fulfil(); - }); - } catch (err) { - reject(err); + return _build({ + legacy: opts.legacy, + bundler, + oncompile: event => { + let banner = `built ${event.type}`; + let c = colors.cyan; + + const { warnings } = event.result; + if (warnings.length > 0) { + banner += ` with ${warnings.length} ${warnings.length === 1 ? 'warning' : 'warnings'}`; + c = colors.yellow; + } + + console.log(); + console.log(c(`┌─${repeat('─', banner.length)}─┐`)); + console.log(c(`│ ${colors.bold(banner) } │`)); + console.log(c(`└─${repeat('─', banner.length)}─┘`)); + + console.log(event.result.print()); } + }, { + dest: locations.dest(), + src: locations.src(), + routes: locations.routes() }); } \ No newline at end of file diff --git a/src/cli/export.ts b/src/cli/export.ts index 214228e78..bcac65e4e 100644 --- a/src/cli/export.ts +++ b/src/cli/export.ts @@ -1,28 +1,31 @@ -import { exporter as _exporter } from '../api/export'; +import { export as _export } from '../api/export'; import colors from 'kleur'; import pb from 'pretty-bytes'; import { locations } from '../config'; import { left_pad } from '../utils'; -export function exporter(export_dir: string, { +export { __export as export }; + +function __export(export_dir: string, { basepath = '', timeout }: { basepath: string, timeout: number | false }) { - return new Promise((fulfil, reject) => { - try { - const emitter = _exporter({ - build: locations.dest(), - static: locations.static(), - dest: export_dir, - basepath, - timeout - }); + return _export({ + build: locations.dest(), + static: locations.static(), + dest: export_dir, + basepath, + timeout, + + oninfo: event => { + console.log(colors.bold.cyan(`> ${event.message}`)); + }, - emitter.on('file', event => { - const size_color = event.size > 150000 ? colors.bold.red : event.size > 50000 ? colors.bold.yellow : colors.bold.gray; + onfile: event => { + const size_color = event.size > 150000 ? colors.bold.red : event.size > 50000 ? colors.bold.yellow : colors.bold.gray; const size_label = size_color(left_pad(pb(event.size), 10)); const file_label = event.status === 200 @@ -30,22 +33,9 @@ export function exporter(export_dir: string, { : colors.bold[event.status >= 400 ? 'red' : 'yellow'](`(${event.status}) ${event.file}`); console.log(`${size_label} ${file_label}`); - }); - - emitter.on('info', event => { - console.log(colors.bold.cyan(`> ${event.message}`)); - }); - - emitter.on('error', event => { - reject(event.error); - }); - - emitter.on('done', event => { - fulfil(); - }); - } catch (err) { - console.log(`${colors.bold.red(`> ${err.message}`)}`); - process.exit(1); } + }).catch(err => { + console.log(`${colors.bold.red(`> ${err.message}`)}`); + process.exit(1); }); } \ No newline at end of file diff --git a/test/apps/basics/__test__.ts b/test/apps/basics/__test__.ts index 5f3553297..842a4b218 100644 --- a/test/apps/basics/__test__.ts +++ b/test/apps/basics/__test__.ts @@ -23,34 +23,21 @@ describe('basics', function() { let goto: (href: string) => Promise; // hooks - before(() => { - return new Promise((fulfil, reject) => { - // TODO this is brittle. Make it unnecessary - process.chdir(__dirname); - process.env.NODE_ENV = 'production'; - - // TODO this API isn't great. Rethink it - const emitter = build({ - bundler: 'rollup' - }, { - src: path.join(__dirname, 'src'), - routes: path.join(__dirname, 'src/routes'), - dest: path.join(__dirname, '__sapper__/build') - }); - - emitter.on('error', reject); - - emitter.on('done', async () => { - try { - runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); - ({ base, page, start, prefetchRoutes, prefetch, goto } = await runner.start()); - - fulfil(); - } catch (err) { - reject(err); - } - }); + before(async () => { + // TODO this is brittle. Make it unnecessary + process.chdir(__dirname); + process.env.NODE_ENV = 'production'; + + await build({ + bundler: 'rollup' + }, { + src: path.join(__dirname, 'src'), + routes: path.join(__dirname, 'src/routes'), + dest: path.join(__dirname, '__sapper__/build') }); + + runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); + ({ base, page, start, prefetchRoutes, prefetch, goto } = await runner.start()); }); after(() => runner.end()); diff --git a/test/apps/credentials/__test__.ts b/test/apps/credentials/__test__.ts index 0f755d2ed..0cc23071b 100644 --- a/test/apps/credentials/__test__.ts +++ b/test/apps/credentials/__test__.ts @@ -17,34 +17,22 @@ describe('credentials', function() { let prefetchRoutes: () => Promise; // hooks - before(() => { - return new Promise((fulfil, reject) => { - // TODO this is brittle. Make it unnecessary - process.chdir(__dirname); - process.env.NODE_ENV = 'production'; - - // TODO this API isn't great. Rethink it - const emitter = build({ - bundler: 'rollup' - }, { - src: path.join(__dirname, 'src'), - routes: path.join(__dirname, 'src/routes'), - dest: path.join(__dirname, '__sapper__/build') - }); - - emitter.on('error', reject); - - emitter.on('done', async () => { - try { - runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); - ({ base, page, start, prefetchRoutes } = await runner.start()); - - fulfil(); - } catch (err) { - reject(err); - } - }); + before(async () => { + // TODO this is brittle. Make it unnecessary + process.chdir(__dirname); + process.env.NODE_ENV = 'production'; + + // TODO this API isn't great. Rethink it + await build({ + bundler: 'rollup' + }, { + src: path.join(__dirname, 'src'), + routes: path.join(__dirname, 'src/routes'), + dest: path.join(__dirname, '__sapper__/build') }); + + runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); + ({ base, page, start, prefetchRoutes } = await runner.start()); }); after(() => runner.end()); diff --git a/test/apps/encoding/__test__.ts b/test/apps/encoding/__test__.ts index c63e02984..c33751511 100644 --- a/test/apps/encoding/__test__.ts +++ b/test/apps/encoding/__test__.ts @@ -17,34 +17,22 @@ describe('encoding', function() { let prefetchRoutes: () => Promise; // hooks - before(() => { - return new Promise((fulfil, reject) => { - // TODO this is brittle. Make it unnecessary - process.chdir(__dirname); - process.env.NODE_ENV = 'production'; - - // TODO this API isn't great. Rethink it - const emitter = build({ - bundler: 'rollup' - }, { - src: path.join(__dirname, 'src'), - routes: path.join(__dirname, 'src/routes'), - dest: path.join(__dirname, '__sapper__/build') - }); - - emitter.on('error', reject); - - emitter.on('done', async () => { - try { - runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); - ({ base, page, start, prefetchRoutes } = await runner.start()); - - fulfil(); - } catch (err) { - reject(err); - } - }); + before(async () => { + // TODO this is brittle. Make it unnecessary + process.chdir(__dirname); + process.env.NODE_ENV = 'production'; + + // TODO this API isn't great. Rethink it + await build({ + bundler: 'rollup' + }, { + src: path.join(__dirname, 'src'), + routes: path.join(__dirname, 'src/routes'), + dest: path.join(__dirname, '__sapper__/build') }); + + runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); + ({ base, page, start, prefetchRoutes } = await runner.start()); }); after(() => runner.end()); diff --git a/test/apps/errors/__test__.ts b/test/apps/errors/__test__.ts index f718b06d5..72b2cfb9e 100644 --- a/test/apps/errors/__test__.ts +++ b/test/apps/errors/__test__.ts @@ -17,34 +17,22 @@ describe('errors', function() { let prefetchRoutes: () => Promise; // hooks - before(() => { - return new Promise((fulfil, reject) => { - // TODO this is brittle. Make it unnecessary - process.chdir(__dirname); - process.env.NODE_ENV = 'production'; - - // TODO this API isn't great. Rethink it - const emitter = build({ - bundler: 'rollup' - }, { - src: path.join(__dirname, 'src'), - routes: path.join(__dirname, 'src/routes'), - dest: path.join(__dirname, '__sapper__/build') - }); - - emitter.on('error', reject); - - emitter.on('done', async () => { - try { - runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); - ({ base, page, start, prefetchRoutes } = await runner.start()); - - fulfil(); - } catch (err) { - reject(err); - } - }); + before(async () => { + // TODO this is brittle. Make it unnecessary + process.chdir(__dirname); + process.env.NODE_ENV = 'production'; + + // TODO this API isn't great. Rethink it + await build({ + bundler: 'rollup' + }, { + src: path.join(__dirname, 'src'), + routes: path.join(__dirname, 'src/routes'), + dest: path.join(__dirname, '__sapper__/build') }); + + runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); + ({ base, page, start, prefetchRoutes } = await runner.start()); }); after(() => runner.end()); diff --git a/test/apps/export/__test__.ts b/test/apps/export/__test__.ts index 9d422e3ee..9c443951b 100644 --- a/test/apps/export/__test__.ts +++ b/test/apps/export/__test__.ts @@ -7,39 +7,28 @@ describe('export', function() { this.timeout(10000); // hooks - before(() => { - return new Promise((fulfil, reject) => { - // TODO this is brittle. Make it unnecessary - process.chdir(__dirname); - process.env.NODE_ENV = 'production'; - - // TODO this API isn't great. Rethink it - const builder = api.build({ - bundler: 'rollup' - }, { - src: path.join(__dirname, 'src'), - routes: path.join(__dirname, 'src/routes'), - dest: path.join(__dirname, '__sapper__/build') - }); - - builder.on('error', reject); - builder.on('done', () => { - // TODO it'd be nice if build and export returned promises. - // not sure how best to combine promise and event emitter - const exporter = api.exporter({ - build: '__sapper__/build', - dest: '__sapper__/export', - static: 'static', - basepath: '', - timeout: 5000 - }); + before(async () => { + // TODO this is brittle. Make it unnecessary + process.chdir(__dirname); + process.env.NODE_ENV = 'production'; + + // TODO this API isn't great. Rethink it + await api.build({ + bundler: 'rollup' + }, { + src: path.join(__dirname, 'src'), + routes: path.join(__dirname, 'src/routes'), + dest: path.join(__dirname, '__sapper__/build') + }); - exporter.on('error', (err: Error) => { - console.error(err); - reject(err); - }); - exporter.on('done', () => fulfil()); - }); + // TODO it'd be nice if build and export returned promises. + // not sure how best to combine promise and event emitter + await api.export({ + build: '__sapper__/build', + dest: '__sapper__/export', + static: 'static', + basepath: '', + timeout: 5000 }); }); diff --git a/test/apps/ignore/__test__.ts b/test/apps/ignore/__test__.ts index b38423ba3..21ccb28af 100644 --- a/test/apps/ignore/__test__.ts +++ b/test/apps/ignore/__test__.ts @@ -12,34 +12,22 @@ describe('ignore', function() { let base: string; // hooks - before(() => { - return new Promise((fulfil, reject) => { - // TODO this is brittle. Make it unnecessary - process.chdir(__dirname); - process.env.NODE_ENV = 'production'; - - // TODO this API isn't great. Rethink it - const emitter = build({ - bundler: 'rollup' - }, { - src: path.join(__dirname, 'src'), - routes: path.join(__dirname, 'src/routes'), - dest: path.join(__dirname, '__sapper__/build') - }); - - emitter.on('error', reject); - - emitter.on('done', async () => { - try { - runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); - ({ base, page } = await runner.start()); - - fulfil(); - } catch (err) { - reject(err); - } - }); + before(async () => { + // TODO this is brittle. Make it unnecessary + process.chdir(__dirname); + process.env.NODE_ENV = 'production'; + + // TODO this API isn't great. Rethink it + await build({ + bundler: 'rollup' + }, { + src: path.join(__dirname, 'src'), + routes: path.join(__dirname, 'src/routes'), + dest: path.join(__dirname, '__sapper__/build') }); + + runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); + ({ base, page } = await runner.start()); }); after(() => runner.end()); diff --git a/test/apps/layout/__test__.ts b/test/apps/layout/__test__.ts index 7edc39034..10047cec7 100644 --- a/test/apps/layout/__test__.ts +++ b/test/apps/layout/__test__.ts @@ -16,34 +16,22 @@ describe('layout', function() { let start: () => Promise; // hooks - before(() => { - return new Promise((fulfil, reject) => { - // TODO this is brittle. Make it unnecessary - process.chdir(__dirname); - process.env.NODE_ENV = 'production'; - - // TODO this API isn't great. Rethink it - const emitter = build({ - bundler: 'rollup' - }, { - src: path.join(__dirname, 'src'), - routes: path.join(__dirname, 'src/routes'), - dest: path.join(__dirname, '__sapper__/build') - }); - - emitter.on('error', reject); - - emitter.on('done', async () => { - try { - runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); - ({ base, page, start } = await runner.start()); - - fulfil(); - } catch (err) { - reject(err); - } - }); + before(async () => { + // TODO this is brittle. Make it unnecessary + process.chdir(__dirname); + process.env.NODE_ENV = 'production'; + + // TODO this API isn't great. Rethink it + await build({ + bundler: 'rollup' + }, { + src: path.join(__dirname, 'src'), + routes: path.join(__dirname, 'src/routes'), + dest: path.join(__dirname, '__sapper__/build') }); + + runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); + ({ base, page, start } = await runner.start()); }); after(() => runner.end()); diff --git a/test/apps/preloading/__test__.ts b/test/apps/preloading/__test__.ts index f11ed980b..f82aba6a0 100644 --- a/test/apps/preloading/__test__.ts +++ b/test/apps/preloading/__test__.ts @@ -19,34 +19,22 @@ describe('preloading', function() { let prefetchRoutes: () => Promise; // hooks - before(() => { - return new Promise((fulfil, reject) => { - // TODO this is brittle. Make it unnecessary - process.chdir(__dirname); - process.env.NODE_ENV = 'production'; - - // TODO this API isn't great. Rethink it - const emitter = build({ - bundler: 'rollup' - }, { - src: path.join(__dirname, 'src'), - routes: path.join(__dirname, 'src/routes'), - dest: path.join(__dirname, '__sapper__/build') - }); - - emitter.on('error', reject); - - emitter.on('done', async () => { - try { - runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); - ({ base, page, start, prefetchRoutes } = await runner.start()); - - fulfil(); - } catch (err) { - reject(err); - } - }); + before(async () => { + // TODO this is brittle. Make it unnecessary + process.chdir(__dirname); + process.env.NODE_ENV = 'production'; + + // TODO this API isn't great. Rethink it + await build({ + bundler: 'rollup' + }, { + src: path.join(__dirname, 'src'), + routes: path.join(__dirname, 'src/routes'), + dest: path.join(__dirname, '__sapper__/build') }); + + runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); + ({ base, page, start, prefetchRoutes } = await runner.start()); }); after(() => runner.end()); diff --git a/test/apps/redirects/__test__.ts b/test/apps/redirects/__test__.ts index 405b657e9..c8bbe692b 100644 --- a/test/apps/redirects/__test__.ts +++ b/test/apps/redirects/__test__.ts @@ -17,34 +17,22 @@ describe('redirects', function() { let prefetchRoutes: () => Promise; // hooks - before(() => { - return new Promise((fulfil, reject) => { - // TODO this is brittle. Make it unnecessary - process.chdir(__dirname); - process.env.NODE_ENV = 'production'; - - // TODO this API isn't great. Rethink it - const emitter = build({ - bundler: 'rollup' - }, { - src: path.join(__dirname, 'src'), - routes: path.join(__dirname, 'src/routes'), - dest: path.join(__dirname, '__sapper__/build') - }); - - emitter.on('error', reject); - - emitter.on('done', async () => { - try { - runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); - ({ base, page, start, prefetchRoutes } = await runner.start()); - - fulfil(); - } catch (err) { - reject(err); - } - }); + before(async () => { + // TODO this is brittle. Make it unnecessary + process.chdir(__dirname); + process.env.NODE_ENV = 'production'; + + // TODO this API isn't great. Rethink it + await build({ + bundler: 'rollup' + }, { + src: path.join(__dirname, 'src'), + routes: path.join(__dirname, 'src/routes'), + dest: path.join(__dirname, '__sapper__/build') }); + + runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); + ({ base, page, start, prefetchRoutes } = await runner.start()); }); after(() => runner.end()); diff --git a/test/apps/scroll/__test__.ts b/test/apps/scroll/__test__.ts index 4e6e40378..76ec21c37 100644 --- a/test/apps/scroll/__test__.ts +++ b/test/apps/scroll/__test__.ts @@ -17,34 +17,22 @@ describe('scroll', function() { let prefetchRoutes: () => Promise; // hooks - before(() => { - return new Promise((fulfil, reject) => { - // TODO this is brittle. Make it unnecessary - process.chdir(__dirname); - process.env.NODE_ENV = 'production'; - - // TODO this API isn't great. Rethink it - const emitter = build({ - bundler: 'rollup' - }, { - src: path.join(__dirname, 'src'), - routes: path.join(__dirname, 'src/routes'), - dest: path.join(__dirname, '__sapper__/build') - }); - - emitter.on('error', reject); - - emitter.on('done', async () => { - try { - runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); - ({ base, page, start, prefetchRoutes } = await runner.start()); - - fulfil(); - } catch (err) { - reject(err); - } - }); + before(async () => { + // TODO this is brittle. Make it unnecessary + process.chdir(__dirname); + process.env.NODE_ENV = 'production'; + + // TODO this API isn't great. Rethink it + await build({ + bundler: 'rollup' + }, { + src: path.join(__dirname, 'src'), + routes: path.join(__dirname, 'src/routes'), + dest: path.join(__dirname, '__sapper__/build') }); + + runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); + ({ base, page, start, prefetchRoutes } = await runner.start()); }); after(() => runner.end()); diff --git a/test/apps/store/__test__.ts b/test/apps/store/__test__.ts index c61f3c031..f9f035c87 100644 --- a/test/apps/store/__test__.ts +++ b/test/apps/store/__test__.ts @@ -15,34 +15,22 @@ describe('store', function() { let start: () => Promise; // hooks - before(() => { - return new Promise((fulfil, reject) => { - // TODO this is brittle. Make it unnecessary - process.chdir(__dirname); - process.env.NODE_ENV = 'production'; - - // TODO this API isn't great. Rethink it - const emitter = build({ - bundler: 'rollup' - }, { - src: path.join(__dirname, 'src'), - routes: path.join(__dirname, 'src/routes'), - dest: path.join(__dirname, '__sapper__/build') - }); - - emitter.on('error', reject); - - emitter.on('done', async () => { - try { - runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); - ({ base, page, start } = await runner.start()); - - fulfil(); - } catch (err) { - reject(err); - } - }); + before(async () => { + // TODO this is brittle. Make it unnecessary + process.chdir(__dirname); + process.env.NODE_ENV = 'production'; + + // TODO this API isn't great. Rethink it + await build({ + bundler: 'rollup' + }, { + src: path.join(__dirname, 'src'), + routes: path.join(__dirname, 'src/routes'), + dest: path.join(__dirname, '__sapper__/build') }); + + runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); + ({ base, page, start } = await runner.start()); }); after(() => runner.end()); diff --git a/test/apps/with-basepath/__test__.ts b/test/apps/with-basepath/__test__.ts index 83a491602..4bc5f1645 100644 --- a/test/apps/with-basepath/__test__.ts +++ b/test/apps/with-basepath/__test__.ts @@ -13,50 +13,32 @@ describe('with-basepath', function() { let base: string; // hooks - before(() => { - return new Promise((fulfil, reject) => { - // TODO this is brittle. Make it unnecessary - process.chdir(__dirname); - process.env.NODE_ENV = 'production'; - - // TODO this API isn't great. Rethink it - const builder = api.build({ - bundler: 'rollup' - }, { - src: path.join(__dirname, 'src'), - routes: path.join(__dirname, 'src/routes'), - dest: path.join(__dirname, '__sapper__/build') - }); - - builder.on('error', reject); - builder.on('done', () => { - // TODO it'd be nice if build and export returned promises. - // not sure how best to combine promise and event emitter - const exporter = api.exporter({ - build: '__sapper__/build', - dest: '__sapper__/export', - static: 'static', - basepath: 'custom-basepath', - timeout: 5000 - }); - - exporter.on('error', (err: Error) => { - console.error(err); - reject(err); - }); - - exporter.on('done', async () => { - try { - runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); - ({ base, page } = await runner.start()); + before(async () => { + // TODO this is brittle. Make it unnecessary + process.chdir(__dirname); + process.env.NODE_ENV = 'production'; + + // TODO this API isn't great. Rethink it + await api.build({ + bundler: 'rollup' + }, { + src: path.join(__dirname, 'src'), + routes: path.join(__dirname, 'src/routes'), + dest: path.join(__dirname, '__sapper__/build') + }); - fulfil(); - } catch (err) { - reject(err); - } - }); - }); + // TODO it'd be nice if build and export returned promises. + // not sure how best to combine promise and event emitter + await api.export({ + build: '__sapper__/build', + dest: '__sapper__/export', + static: 'static', + basepath: 'custom-basepath', + timeout: 5000 }); + + runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); + ({ base, page } = await runner.start()); }); after(() => runner.end()); From 7e4a7d7ab6e8baa3a1903cbedd90947a71d50f86 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 7 Oct 2018 22:20:59 -0400 Subject: [PATCH 2/9] simplify build API --- mocha.opts | 2 +- src/api/build.ts | 50 +++++++++++++------ src/cli/build.ts | 12 +---- test/apps/basics/{__test__.ts => test.ts} | 9 +--- .../apps/credentials/{__test__.ts => test.ts} | 10 +--- test/apps/encoding/{__test__.ts => test.ts} | 10 +--- test/apps/errors/{__test__.ts => test.ts} | 10 +--- test/apps/export/{__test__.ts => test.ts} | 10 +--- test/apps/ignore/{__test__.ts => test.ts} | 10 +--- test/apps/layout/{__test__.ts => test.ts} | 10 +--- test/apps/preloading/{__test__.ts => test.ts} | 9 +--- test/apps/redirects/{__test__.ts => test.ts} | 10 +--- test/apps/scroll/{__test__.ts => test.ts} | 10 +--- test/apps/store/{__test__.ts => test.ts} | 10 +--- .../with-basepath/{__test__.ts => test.ts} | 10 +--- 15 files changed, 49 insertions(+), 133 deletions(-) rename test/apps/basics/{__test__.ts => test.ts} (96%) rename test/apps/credentials/{__test__.ts => test.ts} (85%) rename test/apps/encoding/{__test__.ts => test.ts} (87%) rename test/apps/errors/{__test__.ts => test.ts} (90%) rename test/apps/export/{__test__.ts => test.ts} (81%) rename test/apps/ignore/{__test__.ts => test.ts} (84%) rename test/apps/layout/{__test__.ts => test.ts} (83%) rename test/apps/preloading/{__test__.ts => test.ts} (91%) rename test/apps/redirects/{__test__.ts => test.ts} (87%) rename test/apps/scroll/{__test__.ts => test.ts} (86%) rename test/apps/store/{__test__.ts => test.ts} (77%) rename test/apps/with-basepath/{__test__.ts => test.ts} (87%) diff --git a/mocha.opts b/mocha.opts index e7932784a..c769c5c5e 100644 --- a/mocha.opts +++ b/mocha.opts @@ -2,4 +2,4 @@ --require ts-node/register --recursive test/unit/*/*.ts -test/apps/*/__test__.ts \ No newline at end of file +test/apps/*/test.ts \ No newline at end of file diff --git a/src/api/build.ts b/src/api/build.ts index 2dc2041f4..55c7a37d4 100644 --- a/src/api/build.ts +++ b/src/api/build.ts @@ -9,19 +9,37 @@ import { Dirs } from '../interfaces'; import read_template from '../core/read_template'; import { CompileResult } from '../core/create_compilers/interfaces'; import { noop } from './utils/noop'; +import validate_bundler from '../cli/utils/validate_bundler'; type Opts = { - legacy: boolean; - bundler: 'rollup' | 'webpack'; - oncompile: ({ type, result }: { type: string, result: CompileResult }) => void; + cwd?: string; + src?: string; + routes?: string; + dest?: string; + legacy?: boolean; + bundler?: 'rollup' | 'webpack'; + oncompile?: ({ type, result }: { type: string, result: CompileResult }) => void; }; -export async function build(opts: Opts, dirs: Dirs) { - const { oncompile = noop } = opts; +export async function build({ + cwd = process.cwd(), + src = path.join(cwd, 'src'), + routes = path.join(cwd, 'src/routes'), + dest = path.join(cwd, '__sapper__/build'), - rimraf.sync(path.join(dirs.dest, '**/*')); - mkdirp.sync(`${dirs.dest}/client`); - copy_shimport(dirs.dest); + bundler, + legacy = false, + oncompile = noop +}: Opts = {}) { + bundler = validate_bundler(bundler); + + if (legacy && bundler === 'webpack') { + throw new Error(`Legacy builds are not supported for projects using webpack`); + } + + rimraf.sync(path.join(dest, '**/*')); + mkdirp.sync(`${dest}/client`); + copy_shimport(dest); // minify src/template.html // TODO compile this to a function? could be quicker than str.replace(...).replace(...).replace(...) @@ -34,14 +52,14 @@ export async function build(opts: Opts, dirs: Dirs) { throw error; } - fs.writeFileSync(`${dirs.dest}/template.html`, minify_html(template)); + fs.writeFileSync(`${dest}/template.html`, minify_html(template)); const manifest_data = create_manifest_data(); // create src/manifest/client.js and src/manifest/server.js - create_main_manifests({ bundler: opts.bundler, manifest_data }); + create_main_manifests({ bundler, manifest_data }); - const { client, server, serviceworker } = await create_compilers(opts.bundler); + const { client, server, serviceworker } = await create_compilers(bundler); const client_result = await client.compile(); oncompile({ @@ -49,11 +67,11 @@ export async function build(opts: Opts, dirs: Dirs) { result: client_result }); - const build_info = client_result.to_json(manifest_data, dirs); + const build_info = client_result.to_json(manifest_data, { src, routes, dest }); - if (opts.legacy) { + if (legacy) { process.env.SAPPER_LEGACY_BUILD = 'true'; - const { client } = await create_compilers(opts.bundler); + const { client } = await create_compilers(bundler); const client_result = await client.compile(); @@ -62,12 +80,12 @@ export async function build(opts: Opts, dirs: Dirs) { result: client_result }); - client_result.to_json(manifest_data, dirs); + client_result.to_json(manifest_data, { src, routes, dest }); build_info.legacy_assets = client_result.assets; delete process.env.SAPPER_LEGACY_BUILD; } - fs.writeFileSync(path.join(dirs.dest, 'build.json'), JSON.stringify(build_info)); + fs.writeFileSync(path.join(dest, 'build.json'), JSON.stringify(build_info)); const server_stats = await server.compile(); oncompile({ diff --git a/src/cli/build.ts b/src/cli/build.ts index a118ca1d5..e6ac38b7c 100644 --- a/src/cli/build.ts +++ b/src/cli/build.ts @@ -1,19 +1,12 @@ import { build as _build } from '../api/build'; import colors from 'kleur'; import { locations } from '../config'; -import validate_bundler from './utils/validate_bundler'; import { repeat } from '../utils'; export function build(opts: { bundler?: 'rollup' | 'webpack', legacy?: boolean }) { - const bundler = validate_bundler(opts.bundler); - - if (opts.legacy && bundler === 'webpack') { - throw new Error(`Legacy builds are not supported for projects using webpack`); - } - return _build({ legacy: opts.legacy, - bundler, + bundler: opts.bundler, oncompile: event => { let banner = `built ${event.type}`; let c = colors.cyan; @@ -30,8 +23,7 @@ export function build(opts: { bundler?: 'rollup' | 'webpack', legacy?: boolean } console.log(c(`└─${repeat('─', banner.length)}─┘`)); console.log(event.result.print()); - } - }, { + }, dest: locations.dest(), src: locations.src(), routes: locations.routes() diff --git a/test/apps/basics/__test__.ts b/test/apps/basics/test.ts similarity index 96% rename from test/apps/basics/__test__.ts rename to test/apps/basics/test.ts index 842a4b218..511d506c7 100644 --- a/test/apps/basics/__test__.ts +++ b/test/apps/basics/test.ts @@ -1,4 +1,3 @@ -import * as path from 'path'; import * as assert from 'assert'; import * as puppeteer from 'puppeteer'; import * as http from 'http'; @@ -28,13 +27,7 @@ describe('basics', function() { process.chdir(__dirname); process.env.NODE_ENV = 'production'; - await build({ - bundler: 'rollup' - }, { - src: path.join(__dirname, 'src'), - routes: path.join(__dirname, 'src/routes'), - dest: path.join(__dirname, '__sapper__/build') - }); + await build(); runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); ({ base, page, start, prefetchRoutes, prefetch, goto } = await runner.start()); diff --git a/test/apps/credentials/__test__.ts b/test/apps/credentials/test.ts similarity index 85% rename from test/apps/credentials/__test__.ts rename to test/apps/credentials/test.ts index 0cc23071b..87e557cf8 100644 --- a/test/apps/credentials/__test__.ts +++ b/test/apps/credentials/test.ts @@ -1,4 +1,3 @@ -import * as path from 'path'; import * as assert from 'assert'; import * as puppeteer from 'puppeteer'; import { build } from '../../../api'; @@ -22,14 +21,7 @@ describe('credentials', function() { process.chdir(__dirname); process.env.NODE_ENV = 'production'; - // TODO this API isn't great. Rethink it - await build({ - bundler: 'rollup' - }, { - src: path.join(__dirname, 'src'), - routes: path.join(__dirname, 'src/routes'), - dest: path.join(__dirname, '__sapper__/build') - }); + await build(); runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); ({ base, page, start, prefetchRoutes } = await runner.start()); diff --git a/test/apps/encoding/__test__.ts b/test/apps/encoding/test.ts similarity index 87% rename from test/apps/encoding/__test__.ts rename to test/apps/encoding/test.ts index c33751511..08618ca23 100644 --- a/test/apps/encoding/__test__.ts +++ b/test/apps/encoding/test.ts @@ -1,4 +1,3 @@ -import * as path from 'path'; import * as assert from 'assert'; import * as puppeteer from 'puppeteer'; import { build } from '../../../api'; @@ -22,14 +21,7 @@ describe('encoding', function() { process.chdir(__dirname); process.env.NODE_ENV = 'production'; - // TODO this API isn't great. Rethink it - await build({ - bundler: 'rollup' - }, { - src: path.join(__dirname, 'src'), - routes: path.join(__dirname, 'src/routes'), - dest: path.join(__dirname, '__sapper__/build') - }); + await build(); runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); ({ base, page, start, prefetchRoutes } = await runner.start()); diff --git a/test/apps/errors/__test__.ts b/test/apps/errors/test.ts similarity index 90% rename from test/apps/errors/__test__.ts rename to test/apps/errors/test.ts index 72b2cfb9e..1c72ba1e8 100644 --- a/test/apps/errors/__test__.ts +++ b/test/apps/errors/test.ts @@ -1,4 +1,3 @@ -import * as path from 'path'; import * as assert from 'assert'; import * as puppeteer from 'puppeteer'; import { build } from '../../../api'; @@ -22,14 +21,7 @@ describe('errors', function() { process.chdir(__dirname); process.env.NODE_ENV = 'production'; - // TODO this API isn't great. Rethink it - await build({ - bundler: 'rollup' - }, { - src: path.join(__dirname, 'src'), - routes: path.join(__dirname, 'src/routes'), - dest: path.join(__dirname, '__sapper__/build') - }); + await build(); runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); ({ base, page, start, prefetchRoutes } = await runner.start()); diff --git a/test/apps/export/__test__.ts b/test/apps/export/test.ts similarity index 81% rename from test/apps/export/__test__.ts rename to test/apps/export/test.ts index 9c443951b..23cc2c04a 100644 --- a/test/apps/export/__test__.ts +++ b/test/apps/export/test.ts @@ -1,4 +1,3 @@ -import * as path from 'path'; import * as assert from 'assert'; import { walk } from '../../utils'; import * as api from '../../../api'; @@ -12,14 +11,7 @@ describe('export', function() { process.chdir(__dirname); process.env.NODE_ENV = 'production'; - // TODO this API isn't great. Rethink it - await api.build({ - bundler: 'rollup' - }, { - src: path.join(__dirname, 'src'), - routes: path.join(__dirname, 'src/routes'), - dest: path.join(__dirname, '__sapper__/build') - }); + await api.build(); // TODO it'd be nice if build and export returned promises. // not sure how best to combine promise and event emitter diff --git a/test/apps/ignore/__test__.ts b/test/apps/ignore/test.ts similarity index 84% rename from test/apps/ignore/__test__.ts rename to test/apps/ignore/test.ts index 21ccb28af..95c13bd0e 100644 --- a/test/apps/ignore/__test__.ts +++ b/test/apps/ignore/test.ts @@ -1,4 +1,3 @@ -import * as path from 'path'; import * as assert from 'assert'; import * as puppeteer from 'puppeteer'; import { build } from '../../../api'; @@ -17,14 +16,7 @@ describe('ignore', function() { process.chdir(__dirname); process.env.NODE_ENV = 'production'; - // TODO this API isn't great. Rethink it - await build({ - bundler: 'rollup' - }, { - src: path.join(__dirname, 'src'), - routes: path.join(__dirname, 'src/routes'), - dest: path.join(__dirname, '__sapper__/build') - }); + await build(); runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); ({ base, page } = await runner.start()); diff --git a/test/apps/layout/__test__.ts b/test/apps/layout/test.ts similarity index 83% rename from test/apps/layout/__test__.ts rename to test/apps/layout/test.ts index 10047cec7..a7d72a96c 100644 --- a/test/apps/layout/__test__.ts +++ b/test/apps/layout/test.ts @@ -1,4 +1,3 @@ -import * as path from 'path'; import * as assert from 'assert'; import * as puppeteer from 'puppeteer'; import { build } from '../../../api'; @@ -21,14 +20,7 @@ describe('layout', function() { process.chdir(__dirname); process.env.NODE_ENV = 'production'; - // TODO this API isn't great. Rethink it - await build({ - bundler: 'rollup' - }, { - src: path.join(__dirname, 'src'), - routes: path.join(__dirname, 'src/routes'), - dest: path.join(__dirname, '__sapper__/build') - }); + await build(); runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); ({ base, page, start } = await runner.start()); diff --git a/test/apps/preloading/__test__.ts b/test/apps/preloading/test.ts similarity index 91% rename from test/apps/preloading/__test__.ts rename to test/apps/preloading/test.ts index f82aba6a0..7e321bb66 100644 --- a/test/apps/preloading/__test__.ts +++ b/test/apps/preloading/test.ts @@ -24,14 +24,7 @@ describe('preloading', function() { process.chdir(__dirname); process.env.NODE_ENV = 'production'; - // TODO this API isn't great. Rethink it - await build({ - bundler: 'rollup' - }, { - src: path.join(__dirname, 'src'), - routes: path.join(__dirname, 'src/routes'), - dest: path.join(__dirname, '__sapper__/build') - }); + await build(); runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); ({ base, page, start, prefetchRoutes } = await runner.start()); diff --git a/test/apps/redirects/__test__.ts b/test/apps/redirects/test.ts similarity index 87% rename from test/apps/redirects/__test__.ts rename to test/apps/redirects/test.ts index c8bbe692b..930be5fed 100644 --- a/test/apps/redirects/__test__.ts +++ b/test/apps/redirects/test.ts @@ -1,4 +1,3 @@ -import * as path from 'path'; import * as assert from 'assert'; import * as puppeteer from 'puppeteer'; import { build } from '../../../api'; @@ -22,14 +21,7 @@ describe('redirects', function() { process.chdir(__dirname); process.env.NODE_ENV = 'production'; - // TODO this API isn't great. Rethink it - await build({ - bundler: 'rollup' - }, { - src: path.join(__dirname, 'src'), - routes: path.join(__dirname, 'src/routes'), - dest: path.join(__dirname, '__sapper__/build') - }); + await build(); runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); ({ base, page, start, prefetchRoutes } = await runner.start()); diff --git a/test/apps/scroll/__test__.ts b/test/apps/scroll/test.ts similarity index 86% rename from test/apps/scroll/__test__.ts rename to test/apps/scroll/test.ts index 76ec21c37..fbfee563f 100644 --- a/test/apps/scroll/__test__.ts +++ b/test/apps/scroll/test.ts @@ -1,5 +1,4 @@ import * as assert from 'assert'; -import * as path from 'path'; import * as puppeteer from 'puppeteer'; import { build } from '../../../api'; import { AppRunner } from '../AppRunner'; @@ -22,14 +21,7 @@ describe('scroll', function() { process.chdir(__dirname); process.env.NODE_ENV = 'production'; - // TODO this API isn't great. Rethink it - await build({ - bundler: 'rollup' - }, { - src: path.join(__dirname, 'src'), - routes: path.join(__dirname, 'src/routes'), - dest: path.join(__dirname, '__sapper__/build') - }); + await build(); runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); ({ base, page, start, prefetchRoutes } = await runner.start()); diff --git a/test/apps/store/__test__.ts b/test/apps/store/test.ts similarity index 77% rename from test/apps/store/__test__.ts rename to test/apps/store/test.ts index f9f035c87..d437628c8 100644 --- a/test/apps/store/__test__.ts +++ b/test/apps/store/test.ts @@ -1,4 +1,3 @@ -import * as path from 'path'; import * as assert from 'assert'; import * as puppeteer from 'puppeteer'; import { build } from '../../../api'; @@ -20,14 +19,7 @@ describe('store', function() { process.chdir(__dirname); process.env.NODE_ENV = 'production'; - // TODO this API isn't great. Rethink it - await build({ - bundler: 'rollup' - }, { - src: path.join(__dirname, 'src'), - routes: path.join(__dirname, 'src/routes'), - dest: path.join(__dirname, '__sapper__/build') - }); + await build(); runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); ({ base, page, start } = await runner.start()); diff --git a/test/apps/with-basepath/__test__.ts b/test/apps/with-basepath/test.ts similarity index 87% rename from test/apps/with-basepath/__test__.ts rename to test/apps/with-basepath/test.ts index 4bc5f1645..c23178480 100644 --- a/test/apps/with-basepath/__test__.ts +++ b/test/apps/with-basepath/test.ts @@ -1,4 +1,3 @@ -import * as path from 'path'; import * as assert from 'assert'; import * as puppeteer from 'puppeteer'; import * as api from '../../../api'; @@ -18,14 +17,7 @@ describe('with-basepath', function() { process.chdir(__dirname); process.env.NODE_ENV = 'production'; - // TODO this API isn't great. Rethink it - await api.build({ - bundler: 'rollup' - }, { - src: path.join(__dirname, 'src'), - routes: path.join(__dirname, 'src/routes'), - dest: path.join(__dirname, '__sapper__/build') - }); + await api.build(); // TODO it'd be nice if build and export returned promises. // not sure how best to combine promise and event emitter From c2a650af98f97052194200c8032108b33548dfe0 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 8 Oct 2018 12:03:46 -0400 Subject: [PATCH 3/9] straighten a few things out --- rollup.config.js | 4 +- src/api/build.ts | 26 ++++++--- src/api/dev.ts | 88 ++++++++++++++++++------------ src/api/export.ts | 52 ++++++++++-------- src/api/find_page.ts | 3 +- src/cli.ts | 16 +++--- src/cli/build.ts | 11 ++-- src/cli/export.ts | 14 +++-- src/config.ts | 11 ---- src/config/env.ts | 7 +++ src/{ => config}/rollup.ts | 20 +++---- src/{ => config}/webpack.ts | 16 +++--- src/core/create_compilers/index.ts | 7 ++- src/core/create_manifest_data.ts | 3 +- src/core/create_manifests.ts | 60 +++++++++++++------- src/core/read_template.ts | 3 +- test/apps/export/test.ts | 11 +--- test/apps/with-basepath/test.ts | 8 +-- 18 files changed, 202 insertions(+), 158 deletions(-) delete mode 100644 src/config.ts create mode 100644 src/config/env.ts rename src/{ => config}/rollup.ts (57%) rename src/{ => config}/webpack.ts (66%) diff --git a/rollup.config.js b/rollup.config.js index 8fd7c4b5f..7e13cd6f0 100644 --- a/rollup.config.js +++ b/rollup.config.js @@ -43,8 +43,8 @@ export default [ `src/api.ts`, `src/cli.ts`, `src/core.ts`, - `src/rollup.ts`, - `src/webpack.ts` + `src/config/rollup.ts`, + `src/config/webpack.ts` ], output: { dir: 'dist', diff --git a/src/api/build.ts b/src/api/build.ts index 55c7a37d4..f6cb00f65 100644 --- a/src/api/build.ts +++ b/src/api/build.ts @@ -5,7 +5,6 @@ import rimraf from 'rimraf'; import minify_html from './utils/minify_html'; import { create_compilers, create_main_manifests, create_manifest_data, create_serviceworker_manifest } from '../core'; import { copy_shimport } from './utils/copy_shimport'; -import { Dirs } from '../interfaces'; import read_template from '../core/read_template'; import { CompileResult } from '../core/create_compilers/interfaces'; import { noop } from './utils/noop'; @@ -16,6 +15,8 @@ type Opts = { src?: string; routes?: string; dest?: string; + output?: string; + static_files?: string; legacy?: boolean; bundler?: 'rollup' | 'webpack'; oncompile?: ({ type, result }: { type: string, result: CompileResult }) => void; @@ -25,6 +26,8 @@ export async function build({ cwd = process.cwd(), src = path.join(cwd, 'src'), routes = path.join(cwd, 'src/routes'), + output = path.join(cwd, '__sapper__'), + static_files = path.join(cwd, 'static'), dest = path.join(cwd, '__sapper__/build'), bundler, @@ -43,7 +46,7 @@ export async function build({ // minify src/template.html // TODO compile this to a function? could be quicker than str.replace(...).replace(...).replace(...) - const template = read_template(); + const template = read_template(src); // remove this in a future version if (template.indexOf('%sapper.base%') === -1) { @@ -54,12 +57,20 @@ export async function build({ fs.writeFileSync(`${dest}/template.html`, minify_html(template)); - const manifest_data = create_manifest_data(); + const manifest_data = create_manifest_data(routes); // create src/manifest/client.js and src/manifest/server.js - create_main_manifests({ bundler, manifest_data }); + create_main_manifests({ + bundler, + manifest_data, + src, + dest, + routes, + output, + dev: false + }); - const { client, server, serviceworker } = await create_compilers(bundler); + const { client, server, serviceworker } = await create_compilers(bundler, src, dest, true); const client_result = await client.compile(); oncompile({ @@ -71,7 +82,7 @@ export async function build({ if (legacy) { process.env.SAPPER_LEGACY_BUILD = 'true'; - const { client } = await create_compilers(bundler); + const { client } = await create_compilers(bundler, src, dest, true); const client_result = await client.compile(); @@ -98,7 +109,8 @@ export async function build({ if (serviceworker) { create_serviceworker_manifest({ manifest_data, - client_files: client_result.chunks.map(chunk => `client/${chunk.file}`) + client_files: client_result.chunks.map(chunk => `client/${chunk.file}`), + static_files }); serviceworker_stats = await serviceworker.compile(); diff --git a/src/api/dev.ts b/src/api/dev.ts index 12cd03ed9..fbc40a60f 100644 --- a/src/api/dev.ts +++ b/src/api/dev.ts @@ -5,11 +5,10 @@ import * as child_process from 'child_process'; import * as ports from 'port-authority'; import mkdirp from 'mkdirp'; import rimraf from 'rimraf'; -import { locations } from '../config'; import { EventEmitter } from 'events'; import { create_manifest_data, create_main_manifests, create_compilers, create_serviceworker_manifest } from '../core'; import { Compiler, Compilers } from '../core/create_compilers'; -import { CompileResult, CompileError } from '../core/create_compilers/interfaces'; +import { CompileResult } from '../core/create_compilers/interfaces'; import Deferred from './utils/Deferred'; import * as events from './interfaces'; import validate_bundler from '../cli/utils/validate_bundler'; @@ -18,18 +17,33 @@ import { ManifestData } from '../interfaces'; import read_template from '../core/read_template'; import { noop } from './utils/noop'; +type Opts = { + cwd: string, + src: string, + dest: string, + routes: string, + output: string, + static_files: string, + 'dev-port': number, + live: boolean, + hot: boolean, + 'devtools-port': number, + bundler?: 'rollup' | 'webpack', + port: number +}; + export function dev(opts) { return new Watcher(opts); } class Watcher extends EventEmitter { - bundler: string; + bundler: 'rollup' | 'webpack'; dirs: { src: string; dest: string; routes: string; - rollup: string; - webpack: string; + output: string; + static_files: string; } port: number; closed: boolean; @@ -55,34 +69,23 @@ class Watcher extends EventEmitter { } constructor({ - src = locations.src(), - dest = locations.dest(), - routes = locations.routes(), + cwd = process.cwd(), + src = path.join(cwd, 'src'), + routes = path.join(cwd, 'src/routes'), + output = path.join(cwd, '__sapper__'), + static_files = path.join(cwd, 'static'), + dest = path.join(cwd, '__sapper__/dev'), 'dev-port': dev_port, live, hot, 'devtools-port': devtools_port, bundler, - webpack = 'webpack', - rollup = 'rollup', port = +process.env.PORT - }: { - src: string, - dest: string, - routes: string, - 'dev-port': number, - live: boolean, - hot: boolean, - 'devtools-port': number, - bundler?: string, - webpack: string, - rollup: string, - port: number - }) { + }: Opts) { super(); this.bundler = validate_bundler(bundler); - this.dirs = { src, dest, routes, webpack, rollup }; + this.dirs = { src, dest, routes, output, static_files }; this.port = port; this.closed = false; @@ -102,7 +105,7 @@ class Watcher extends EventEmitter { }; // remove this in a future version - const template = read_template(); + const template = read_template(src); if (template.indexOf('%sapper.base%') === -1) { const error = new Error(`As of Sapper v0.10, your template.html file must include %sapper.base% in the `); error.code = `missing-sapper-base`; @@ -130,7 +133,7 @@ class Watcher extends EventEmitter { this.port = await ports.find(3000); } - const { dest } = this.dirs; + const { src, dest, routes, output, static_files } = this.dirs; rimraf.sync(dest); mkdirp.sync(`${dest}/client`); if (this.bundler === 'rollup') copy_shimport(dest); @@ -143,8 +146,14 @@ class Watcher extends EventEmitter { let manifest_data: ManifestData; try { - manifest_data = create_manifest_data(); - create_main_manifests({ bundler: this.bundler, manifest_data, dev_port: this.dev_port }); + manifest_data = create_manifest_data(routes); + create_main_manifests({ + bundler: this.bundler, + manifest_data, + dev: true, + dev_port: this.dev_port, + src, dest, routes, output + }); } catch (err) { this.emit('fatal', { message: err.message @@ -156,7 +165,7 @@ class Watcher extends EventEmitter { this.filewatchers.push( watch_dir( - locations.routes(), + routes, ({ path: file, stats }) => { if (stats.isDirectory()) { return path.basename(file)[0] !== '_'; @@ -165,8 +174,14 @@ class Watcher extends EventEmitter { }, () => { try { - const new_manifest_data = create_manifest_data(); - create_main_manifests({ bundler: this.bundler, manifest_data, dev_port: this.dev_port }); + const new_manifest_data = create_manifest_data(routes); + create_main_manifests({ + bundler: this.bundler, + manifest_data, // TODO is this right? not new_manifest_data? + dev: true, + dev_port: this.dev_port, + src, dest, routes, output + }); manifest_data = new_manifest_data; } catch (err) { @@ -177,7 +192,7 @@ class Watcher extends EventEmitter { } ), - fs.watch(`${locations.src()}/template.html`, () => { + fs.watch(`${src}/template.html`, () => { this.dev_server.send({ action: 'reload' }); @@ -187,7 +202,7 @@ class Watcher extends EventEmitter { let deferred = new Deferred(); // TODO watch the configs themselves? - const compilers: Compilers = await create_compilers(this.bundler, this.dirs); + const compilers: Compilers = await create_compilers(this.bundler, src, dest, false); let log = ''; @@ -313,7 +328,8 @@ class Watcher extends EventEmitter { create_serviceworker_manifest({ manifest_data, - client_files + client_files, + static_files }); deferred.fulfil(); @@ -461,7 +477,7 @@ function watch_dir( filter: ({ path, stats }: { path: string, stats: fs.Stats }) => boolean, callback: () => void ) { - let watch; + let watch: any; let closed = false; import('cheap-watch').then(CheapWatch => { @@ -469,7 +485,7 @@ function watch_dir( watch = new CheapWatch({ dir, filter, debounce: 50 }); - watch.on('+', ({ isNew }) => { + watch.on('+', ({ isNew }: { isNew: boolean }) => { if (isNew) callback(); }); diff --git a/src/api/export.ts b/src/api/export.ts index b12f67752..81f9dcdcf 100644 --- a/src/api/export.ts +++ b/src/api/export.ts @@ -10,13 +10,14 @@ import Deferred from './utils/Deferred'; import { noop } from './utils/noop'; type Opts = { - build: string, - dest: string, - static: string, + build_dir?: string, + export_dir?: string, + cwd?: string, + static?: string, basepath?: string, - timeout: number | false, - oninfo: ({ message }: { message: string }) => void; - onfile: ({ file, size, status }: { file: string, size: number, status: number }) => void; + timeout?: number | false, + oninfo?: ({ message }: { message: string }) => void; + onfile?: ({ file, size, status }: { file: string, size: number, status: number }) => void; }; function resolve(from: string, to: string) { @@ -27,23 +28,28 @@ type URL = url.UrlWithStringQuery; export { _export as export }; -async function _export(opts: Opts) { - const { oninfo = noop, onfile = noop } = opts; - - const export_dir = path.join(opts.dest, opts.basepath); - +async function _export({ + cwd = process.cwd(), + static: static_files = path.join(cwd, 'static'), + build_dir = path.join(cwd, '__sapper__/build'), + basepath = '', + export_dir = path.join(cwd, '__sapper__/export', basepath), + timeout = 5000, + oninfo = noop, + onfile = noop +}: Opts = {}) { // Prep output directory sander.rimrafSync(export_dir); - sander.copydirSync(opts.static).to(export_dir); - sander.copydirSync(opts.build, 'client').to(export_dir, 'client'); + sander.copydirSync(static_files).to(export_dir); + sander.copydirSync(build_dir, 'client').to(export_dir, 'client'); - if (sander.existsSync(opts.build, 'service-worker.js')) { - sander.copyFileSync(opts.build, 'service-worker.js').to(export_dir, 'service-worker.js'); + if (sander.existsSync(build_dir, 'service-worker.js')) { + sander.copyFileSync(build_dir, 'service-worker.js').to(export_dir, 'service-worker.js'); } - if (sander.existsSync(opts.build, 'service-worker.js.map')) { - sander.copyFileSync(opts.build, 'service-worker.js.map').to(export_dir, 'service-worker.js.map'); + if (sander.existsSync(build_dir, 'service-worker.js.map')) { + sander.copyFileSync(build_dir, 'service-worker.js.map').to(export_dir, 'service-worker.js.map'); } const port = await ports.find(3000); @@ -52,19 +58,19 @@ async function _export(opts: Opts) { const host = `localhost:${port}`; const origin = `${protocol}//${host}`; - const root = resolve(origin, opts.basepath || ''); + const root = resolve(origin, basepath); if (!root.href.endsWith('/')) root.href += '/'; oninfo({ message: `Crawling ${root.href}` }); - const proc = child_process.fork(path.resolve(`${opts.build}/server/server.js`), [], { + const proc = child_process.fork(path.resolve(`${build_dir}/server/server.js`), [], { cwd: process.cwd(), env: Object.assign({ PORT: port, NODE_ENV: 'production', - SAPPER_DEST: opts.build, + SAPPER_DEST: build_dir, SAPPER_EXPORT: 'true' }, process.env) }); @@ -107,9 +113,9 @@ async function _export(opts: Opts) { seen.add(pathname); const timeout_deferred = new Deferred(); - const timeout = setTimeout(() => { + const the_timeout = setTimeout(() => { timeout_deferred.reject(new Error(`Timed out waiting for ${url.href}`)); - }, opts.timeout); + }, timeout); const r = await Promise.race([ fetch(url.href, { @@ -118,7 +124,7 @@ async function _export(opts: Opts) { timeout_deferred.promise ]); - clearTimeout(timeout); // prevent it hanging at the end + clearTimeout(the_timeout); // prevent it hanging at the end let type = r.headers.get('Content-Type'); let body = await r.text(); diff --git a/src/api/find_page.ts b/src/api/find_page.ts index e986176f9..6f7cac91f 100644 --- a/src/api/find_page.ts +++ b/src/api/find_page.ts @@ -1,7 +1,6 @@ -import { locations } from '../config'; import { create_manifest_data } from '../core'; -export function find_page(pathname: string, cwd = locations.routes()) { +export function find_page(pathname: string, cwd = 'src/routes') { const { pages } = create_manifest_data(cwd); for (let i = 0; i < pages.length; i += 1) { diff --git a/src/cli.ts b/src/cli.ts index e44e5d317..676f71309 100755 --- a/src/cli.ts +++ b/src/cli.ts @@ -36,18 +36,17 @@ prog.command('build [dest]') .action(async (dest = '__sapper__/build', opts: { port: string, legacy: boolean, - bundler?: string + bundler?: 'rollup' | 'webpack' }) => { console.log(`> Building...`); process.env.NODE_ENV = process.env.NODE_ENV || 'production'; - process.env.SAPPER_DEST = dest; const start = Date.now(); try { const { build } = await import('./cli/build'); - await build(opts); + await build(Object.assign({ dest }, opts)); const launcher = path.resolve(dest, 'index.js'); @@ -88,13 +87,16 @@ prog.command('export [dest]') .action(async (dest = '__sapper__/export', opts: { build: boolean, legacy: boolean, - bundler?: string, + bundler?: 'rollup' | 'webpack', 'build-dir': string, basepath?: string, timeout: number | false }) => { process.env.NODE_ENV = 'production'; - process.env.SAPPER_DEST = opts['build-dir']; + + Object.assign(opts, { + build_dir: opts['build-dir'] + }); const start = Date.now(); @@ -106,8 +108,8 @@ prog.command('export [dest]') console.error(`\n> Built in ${elapsed(start)}`); } - const { exporter } = await import('./cli/export'); - await exporter(dest, opts); + const { export: _export } = await import('./cli/export'); + await _export(dest, opts); console.error(`\n> Finished in ${elapsed(start)}. Type ${colors.bold.cyan(`npx serve ${dest}`)} to run the app.`); } catch (err) { console.error(colors.bold.red(`> ${err.message}`)); diff --git a/src/cli/build.ts b/src/cli/build.ts index e6ac38b7c..e95c5d535 100644 --- a/src/cli/build.ts +++ b/src/cli/build.ts @@ -1,9 +1,11 @@ +import * as path from 'path'; import { build as _build } from '../api/build'; import colors from 'kleur'; -import { locations } from '../config'; import { repeat } from '../utils'; export function build(opts: { bundler?: 'rollup' | 'webpack', legacy?: boolean }) { + const cwd = path.resolve(process.env.SAPPER_BASE || ''); + return _build({ legacy: opts.legacy, bundler: opts.bundler, @@ -24,8 +26,9 @@ export function build(opts: { bundler?: 'rollup' | 'webpack', legacy?: boolean } console.log(event.result.print()); }, - dest: locations.dest(), - src: locations.src(), - routes: locations.routes() + cwd, + src: path.resolve(cwd, process.env.SAPPER_SRC || 'src'), + routes: path.resolve(cwd, process.env.SAPPER_ROUTES || 'src/routes'), + dest: path.resolve(cwd, process.env.SAPPER_DEST || '__sapper__/build') }); } \ No newline at end of file diff --git a/src/cli/export.ts b/src/cli/export.ts index bcac65e4e..cba77023a 100644 --- a/src/cli/export.ts +++ b/src/cli/export.ts @@ -1,22 +1,26 @@ +import * as path from 'path'; import { export as _export } from '../api/export'; import colors from 'kleur'; import pb from 'pretty-bytes'; -import { locations } from '../config'; import { left_pad } from '../utils'; export { __export as export }; function __export(export_dir: string, { + build_dir, basepath = '', timeout }: { - basepath: string, + build_dir: string, + basepath?: string, timeout: number | false }) { + const cwd = path.resolve(process.env.SAPPER_BASE || ''); + return _export({ - build: locations.dest(), - static: locations.static(), - dest: export_dir, + static: path.resolve(cwd, process.env.SAPPER_STATIC || 'static'), + build_dir, + export_dir, basepath, timeout, diff --git a/src/config.ts b/src/config.ts deleted file mode 100644 index 6ee9941fd..000000000 --- a/src/config.ts +++ /dev/null @@ -1,11 +0,0 @@ -import * as path from 'path'; - -export const dev = () => process.env.NODE_ENV !== 'production'; - -export const locations = { - base: () => path.resolve(process.env.SAPPER_BASE || ''), - src: () => path.resolve(process.env.SAPPER_BASE || '', process.env.SAPPER_SRC || 'src'), - static: () => path.resolve(process.env.SAPPER_BASE || '', process.env.SAPPER_STATIC || 'static'), - routes: () => path.resolve(process.env.SAPPER_BASE || '', process.env.SAPPER_ROUTES || 'src/routes'), - dest: () => path.resolve(process.env.SAPPER_BASE || '', process.env.SAPPER_DEST || `__sapper__/${dev() ? 'dev' : 'build'}`) -}; \ No newline at end of file diff --git a/src/config/env.ts b/src/config/env.ts new file mode 100644 index 000000000..6a19fe89b --- /dev/null +++ b/src/config/env.ts @@ -0,0 +1,7 @@ +export let dev: boolean; +export let src: string; +export let dest: string; + +export const set_dev = (_: boolean) => dev = _; +export const set_src = (_: string) => src = _; +export const set_dest = (_: string) => dest = _; \ No newline at end of file diff --git a/src/rollup.ts b/src/config/rollup.ts similarity index 57% rename from src/rollup.ts rename to src/config/rollup.ts index 8dd591ead..fd3502678 100644 --- a/src/rollup.ts +++ b/src/config/rollup.ts @@ -1,15 +1,15 @@ -import { locations, dev } from './config'; +import { dev, src, dest } from './env'; export default { - dev: dev(), + dev, client: { input: () => { - return `${locations.src()}/client.js` + return `${src}/client.js` }, output: () => { - let dir = `${locations.dest()}/client`; + let dir = `${dest}/client`; if (process.env.SAPPER_LEGACY_BUILD) dir += `/legacy`; return { @@ -17,7 +17,7 @@ export default { entryFileNames: '[name].[hash].js', chunkFileNames: '[name].[hash].js', format: 'esm', - sourcemap: dev() + sourcemap: dev }; } }, @@ -25,27 +25,27 @@ export default { server: { input: () => { return { - server: `${locations.src()}/server.js` + server: `${src}/server.js` }; }, output: () => { return { - dir: `${locations.dest()}/server`, + dir: `${dest}/server`, format: 'cjs', - sourcemap: dev() + sourcemap: dev }; } }, serviceworker: { input: () => { - return `${locations.src()}/service-worker.js`; + return `${src}/service-worker.js`; }, output: () => { return { - file: `${locations.dest()}/service-worker.js`, + file: `${dest}/service-worker.js`, format: 'iife' } } diff --git a/src/webpack.ts b/src/config/webpack.ts similarity index 66% rename from src/webpack.ts rename to src/config/webpack.ts index c3424e0c6..06d821d64 100644 --- a/src/webpack.ts +++ b/src/config/webpack.ts @@ -1,18 +1,18 @@ -import { locations, dev } from './config'; +import { dev, src, dest } from './env'; export default { - dev: dev(), + dev, client: { entry: () => { return { - main: `${locations.src()}/client` + main: `${src}/client` }; }, output: () => { return { - path: `${locations.dest()}/client`, + path: `${dest}/client`, filename: '[hash]/[name].js', chunkFilename: '[hash]/[name].[id].js', publicPath: `client/` @@ -23,13 +23,13 @@ export default { server: { entry: () => { return { - server: `${locations.src()}/server` + server: `${src}/server` }; }, output: () => { return { - path: `${locations.dest()}/server`, + path: `${dest}/server`, filename: '[name].js', chunkFilename: '[hash]/[name].[id].js', libraryTarget: 'commonjs2' @@ -40,13 +40,13 @@ export default { serviceworker: { entry: () => { return { - 'service-worker': `${locations.src()}/service-worker` + 'service-worker': `${src}/service-worker` }; }, output: () => { return { - path: locations.dest(), + path: dest, filename: '[name].js', chunkFilename: '[name].[id].[hash].js' } diff --git a/src/core/create_compilers/index.ts b/src/core/create_compilers/index.ts index 3edee5d4c..e80903c3e 100644 --- a/src/core/create_compilers/index.ts +++ b/src/core/create_compilers/index.ts @@ -1,6 +1,7 @@ import * as path from 'path'; import RollupCompiler from './RollupCompiler'; import { WebpackCompiler } from './WebpackCompiler'; +import { set_dev, set_src, set_dest } from '../../config/env'; export type Compiler = RollupCompiler | WebpackCompiler; @@ -10,7 +11,11 @@ export type Compilers = { serviceworker?: Compiler; } -export default async function create_compilers(bundler: 'rollup' | 'webpack'): Promise { +export default async function create_compilers(bundler: 'rollup' | 'webpack', src: string, dest: string, dev: boolean): Promise { + set_dev(dev); + set_src(src); + set_dest(dest); + if (bundler === 'rollup') { const config = await RollupCompiler.load_config(); validate_config(config, 'rollup'); diff --git a/src/core/create_manifest_data.ts b/src/core/create_manifest_data.ts index 67ff485d4..c29c59a8c 100644 --- a/src/core/create_manifest_data.ts +++ b/src/core/create_manifest_data.ts @@ -1,10 +1,9 @@ import * as fs from 'fs'; import * as path from 'path'; -import { locations } from '../config'; import { Page, PageComponent, ServerRoute, ManifestData } from '../interfaces'; import { posixify, reserved_words } from './utils'; -export default function create_manifest_data(cwd = locations.routes()): ManifestData { +export default function create_manifest_data(cwd: string): ManifestData { // TODO remove in a future version if (!fs.existsSync(cwd)) { throw new Error(`As of Sapper 0.21, the routes/ directory should become src/routes/`); diff --git a/src/core/create_manifests.ts b/src/core/create_manifests.ts index 6354dfb53..1f6dff47a 100644 --- a/src/core/create_manifests.ts +++ b/src/core/create_manifests.ts @@ -2,40 +2,54 @@ import * as fs from 'fs'; import * as path from 'path'; import glob from 'tiny-glob/sync.js'; import { posixify, stringify, write_if_changed } from './utils'; -import { dev, locations } from '../config'; -import { Page, PageComponent, ServerRoute, ManifestData } from '../interfaces'; - -export function create_main_manifests({ bundler, manifest_data, dev_port }: { +import { dev } from '../config/env'; +import { Page, PageComponent, ManifestData } from '../interfaces'; + +export function create_main_manifests({ + bundler, + manifest_data, + dev_port, + dev, + src, + dest, + routes, + output +}: { bundler: string, manifest_data: ManifestData; dev_port?: number; + dev: boolean; + src: string; + dest: string; + routes: string; + output: string }) { - const manifest_dir = path.resolve('__sapper__'); - if (!fs.existsSync(manifest_dir)) fs.mkdirSync(manifest_dir); + if (!fs.existsSync(output)) fs.mkdirSync(output); - const path_to_routes = path.relative(manifest_dir, locations.routes()); + const path_to_routes = path.relative(output, routes); - const client_manifest = generate_client(manifest_data, path_to_routes, bundler, dev_port); - const server_manifest = generate_server(manifest_data, path_to_routes); + const client_manifest = generate_client(manifest_data, path_to_routes, bundler, dev, dev_port); + const server_manifest = generate_server(manifest_data, path_to_routes, src, dest, dev); write_if_changed( - `${manifest_dir}/_layout.html`, + `${output}/_layout.html`, `` ); - write_if_changed(`${manifest_dir}/client.js`, client_manifest); - write_if_changed(`${manifest_dir}/server.js`, server_manifest); + write_if_changed(`${output}/client.js`, client_manifest); + write_if_changed(`${output}/server.js`, server_manifest); } -export function create_serviceworker_manifest({ manifest_data, client_files }: { +export function create_serviceworker_manifest({ manifest_data, client_files, static_files }: { manifest_data: ManifestData; client_files: string[]; + static_files: string; }) { let files; - // TODO remove in a future version - if (fs.existsSync(locations.static())) { - files = glob('**', { cwd: locations.static(), filesOnly: true }); + if (fs.existsSync(static_files)) { + files = glob('**', { cwd: static_files, filesOnly: true }); } else { + // TODO remove in a future version if (fs.existsSync('assets')) { throw new Error(`As of Sapper 0.21, the assets/ directory should become static/`); } @@ -62,6 +76,7 @@ function generate_client( manifest_data: ManifestData, path_to_routes: string, bundler: string, + dev: boolean, dev_port?: number ) { const template_file = path.resolve(__dirname, '../templates/client.js'); @@ -120,7 +135,7 @@ function generate_client( let footer = ''; - if (dev()) { + if (dev) { const sapper_dev_client = posixify( path.resolve(__dirname, '../sapper-dev-client.js') ); @@ -145,7 +160,10 @@ function generate_client( function generate_server( manifest_data: ManifestData, - path_to_routes: string + path_to_routes: string, + src: string, + dest: string, + dev: boolean ) { const template_file = path.resolve(__dirname, '../templates/server.js'); const template = fs.readFileSync(template_file, 'utf-8'); @@ -206,13 +224,13 @@ function generate_server( error };`.replace(/^\t\t/gm, '').trim(); - const build_dir = path.relative(process.cwd(), locations.dest()); - const src_dir = path.relative(process.cwd(), locations.src()); + const build_dir = path.relative(process.cwd(), dest); + const src_dir = path.relative(process.cwd(), src); return `// This file is generated by Sapper — do not edit it!\n` + template .replace('__BUILD__DIR__', JSON.stringify(build_dir)) .replace('__SRC__DIR__', JSON.stringify(src_dir)) - .replace('__DEV__', dev() ? 'true' : 'false') + .replace('__DEV__', dev ? 'true' : 'false') .replace(/const manifest = __MANIFEST__;/, code); } diff --git a/src/core/read_template.ts b/src/core/read_template.ts index 330f33e05..afd0f734b 100644 --- a/src/core/read_template.ts +++ b/src/core/read_template.ts @@ -1,7 +1,6 @@ import * as fs from 'fs'; -import { locations } from '../config'; -export default function read_template(dir = locations.src()) { +export default function read_template(dir: string) { try { return fs.readFileSync(`${dir}/template.html`, 'utf-8'); } catch (err) { diff --git a/test/apps/export/test.ts b/test/apps/export/test.ts index 23cc2c04a..258cb0320 100644 --- a/test/apps/export/test.ts +++ b/test/apps/export/test.ts @@ -12,16 +12,7 @@ describe('export', function() { process.env.NODE_ENV = 'production'; await api.build(); - - // TODO it'd be nice if build and export returned promises. - // not sure how best to combine promise and event emitter - await api.export({ - build: '__sapper__/build', - dest: '__sapper__/export', - static: 'static', - basepath: '', - timeout: 5000 - }); + await api.export(); }); it('crawls a site', () => { diff --git a/test/apps/with-basepath/test.ts b/test/apps/with-basepath/test.ts index c23178480..1284f0275 100644 --- a/test/apps/with-basepath/test.ts +++ b/test/apps/with-basepath/test.ts @@ -19,14 +19,8 @@ describe('with-basepath', function() { await api.build(); - // TODO it'd be nice if build and export returned promises. - // not sure how best to combine promise and event emitter await api.export({ - build: '__sapper__/build', - dest: '__sapper__/export', - static: 'static', - basepath: 'custom-basepath', - timeout: 5000 + basepath: 'custom-basepath' }); runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); From 74f45416437131553a03a81c0fe4ba4a644d846f Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 8 Oct 2018 13:21:41 -0400 Subject: [PATCH 4/9] end reliance on process.cwd() --- src/api/build.ts | 5 +++-- src/api/dev.ts | 11 ++++++----- src/api/export.ts | 2 +- src/core/create_compilers/RollupCompiler.ts | 6 +++--- src/core/create_compilers/index.ts | 12 +++++++++--- src/core/create_manifests.ts | 10 ++++++---- test/apps/basics/test.ts | 6 +----- test/apps/credentials/test.ts | 6 +----- test/apps/encoding/test.ts | 6 +----- test/apps/errors/test.ts | 6 +----- test/apps/export/test.ts | 10 +++------- test/apps/ignore/test.ts | 6 +----- test/apps/layout/test.ts | 6 +----- test/apps/preloading/test.ts | 6 +----- test/apps/redirects/test.ts | 6 +----- test/apps/scroll/test.ts | 6 +----- test/apps/store/test.ts | 6 +----- test/apps/with-basepath/test.ts | 9 +++------ 18 files changed, 44 insertions(+), 81 deletions(-) diff --git a/src/api/build.ts b/src/api/build.ts index f6cb00f65..4f56d8620 100644 --- a/src/api/build.ts +++ b/src/api/build.ts @@ -63,6 +63,7 @@ export async function build({ create_main_manifests({ bundler, manifest_data, + cwd, src, dest, routes, @@ -70,7 +71,7 @@ export async function build({ dev: false }); - const { client, server, serviceworker } = await create_compilers(bundler, src, dest, true); + const { client, server, serviceworker } = await create_compilers(bundler, cwd, src, dest, true); const client_result = await client.compile(); oncompile({ @@ -82,7 +83,7 @@ export async function build({ if (legacy) { process.env.SAPPER_LEGACY_BUILD = 'true'; - const { client } = await create_compilers(bundler, src, dest, true); + const { client } = await create_compilers(bundler, cwd, src, dest, true); const client_result = await client.compile(); diff --git a/src/api/dev.ts b/src/api/dev.ts index fbc40a60f..722d9c266 100644 --- a/src/api/dev.ts +++ b/src/api/dev.ts @@ -39,6 +39,7 @@ export function dev(opts) { class Watcher extends EventEmitter { bundler: 'rollup' | 'webpack'; dirs: { + cwd: string; src: string; dest: string; routes: string; @@ -85,7 +86,7 @@ class Watcher extends EventEmitter { super(); this.bundler = validate_bundler(bundler); - this.dirs = { src, dest, routes, output, static_files }; + this.dirs = { cwd, src, dest, routes, output, static_files }; this.port = port; this.closed = false; @@ -133,7 +134,7 @@ class Watcher extends EventEmitter { this.port = await ports.find(3000); } - const { src, dest, routes, output, static_files } = this.dirs; + const { cwd, src, dest, routes, output, static_files } = this.dirs; rimraf.sync(dest); mkdirp.sync(`${dest}/client`); if (this.bundler === 'rollup') copy_shimport(dest); @@ -152,7 +153,7 @@ class Watcher extends EventEmitter { manifest_data, dev: true, dev_port: this.dev_port, - src, dest, routes, output + cwd, src, dest, routes, output }); } catch (err) { this.emit('fatal', { @@ -180,7 +181,7 @@ class Watcher extends EventEmitter { manifest_data, // TODO is this right? not new_manifest_data? dev: true, dev_port: this.dev_port, - src, dest, routes, output + cwd, src, dest, routes, output }); manifest_data = new_manifest_data; @@ -202,7 +203,7 @@ class Watcher extends EventEmitter { let deferred = new Deferred(); // TODO watch the configs themselves? - const compilers: Compilers = await create_compilers(this.bundler, src, dest, false); + const compilers: Compilers = await create_compilers(this.bundler, cwd, src, dest, false); let log = ''; diff --git a/src/api/export.ts b/src/api/export.ts index 81f9dcdcf..b5d1d01de 100644 --- a/src/api/export.ts +++ b/src/api/export.ts @@ -66,7 +66,7 @@ async function _export({ }); const proc = child_process.fork(path.resolve(`${build_dir}/server/server.js`), [], { - cwd: process.cwd(), + cwd, env: Object.assign({ PORT: port, NODE_ENV: 'production', diff --git a/src/core/create_compilers/RollupCompiler.ts b/src/core/create_compilers/RollupCompiler.ts index b90087270..f750e42f3 100644 --- a/src/core/create_compilers/RollupCompiler.ts +++ b/src/core/create_compilers/RollupCompiler.ts @@ -135,10 +135,10 @@ export default class RollupCompiler { }); } - static async load_config() { - if (!rollup) rollup = relative('rollup', process.cwd()); + static async load_config(cwd: string) { + if (!rollup) rollup = relative('rollup', cwd); - const input = path.resolve('rollup.config.js'); + const input = path.resolve(cwd, 'rollup.config.js'); const bundle = await rollup.rollup({ input, diff --git a/src/core/create_compilers/index.ts b/src/core/create_compilers/index.ts index e80903c3e..43d9ddb60 100644 --- a/src/core/create_compilers/index.ts +++ b/src/core/create_compilers/index.ts @@ -11,13 +11,19 @@ export type Compilers = { serviceworker?: Compiler; } -export default async function create_compilers(bundler: 'rollup' | 'webpack', src: string, dest: string, dev: boolean): Promise { +export default async function create_compilers( + bundler: 'rollup' | 'webpack', + cwd: string, + src: string, + dest: string, + dev: boolean +): Promise { set_dev(dev); set_src(src); set_dest(dest); if (bundler === 'rollup') { - const config = await RollupCompiler.load_config(); + const config = await RollupCompiler.load_config(cwd); validate_config(config, 'rollup'); normalize_rollup_config(config.client); @@ -35,7 +41,7 @@ export default async function create_compilers(bundler: 'rollup' | 'webpack', sr } if (bundler === 'webpack') { - const config = require(path.resolve('webpack.config.js')); + const config = require(path.resolve(cwd, 'webpack.config.js')); validate_config(config, 'webpack'); return { diff --git a/src/core/create_manifests.ts b/src/core/create_manifests.ts index 1f6dff47a..3a25df0f5 100644 --- a/src/core/create_manifests.ts +++ b/src/core/create_manifests.ts @@ -2,7 +2,6 @@ import * as fs from 'fs'; import * as path from 'path'; import glob from 'tiny-glob/sync.js'; import { posixify, stringify, write_if_changed } from './utils'; -import { dev } from '../config/env'; import { Page, PageComponent, ManifestData } from '../interfaces'; export function create_main_manifests({ @@ -10,6 +9,7 @@ export function create_main_manifests({ manifest_data, dev_port, dev, + cwd, src, dest, routes, @@ -19,6 +19,7 @@ export function create_main_manifests({ manifest_data: ManifestData; dev_port?: number; dev: boolean; + cwd: string; src: string; dest: string; routes: string; @@ -29,7 +30,7 @@ export function create_main_manifests({ const path_to_routes = path.relative(output, routes); const client_manifest = generate_client(manifest_data, path_to_routes, bundler, dev, dev_port); - const server_manifest = generate_server(manifest_data, path_to_routes, src, dest, dev); + const server_manifest = generate_server(manifest_data, path_to_routes, cwd, src, dest, dev); write_if_changed( `${output}/_layout.html`, @@ -161,6 +162,7 @@ function generate_client( function generate_server( manifest_data: ManifestData, path_to_routes: string, + cwd: string, src: string, dest: string, dev: boolean @@ -224,8 +226,8 @@ function generate_server( error };`.replace(/^\t\t/gm, '').trim(); - const build_dir = path.relative(process.cwd(), dest); - const src_dir = path.relative(process.cwd(), src); + const build_dir = path.relative(cwd, dest); + const src_dir = path.relative(cwd, src); return `// This file is generated by Sapper — do not edit it!\n` + template .replace('__BUILD__DIR__', JSON.stringify(build_dir)) diff --git a/test/apps/basics/test.ts b/test/apps/basics/test.ts index 511d506c7..7190fd905 100644 --- a/test/apps/basics/test.ts +++ b/test/apps/basics/test.ts @@ -23,11 +23,7 @@ describe('basics', function() { // hooks before(async () => { - // TODO this is brittle. Make it unnecessary - process.chdir(__dirname); - process.env.NODE_ENV = 'production'; - - await build(); + await build({ cwd: __dirname }); runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); ({ base, page, start, prefetchRoutes, prefetch, goto } = await runner.start()); diff --git a/test/apps/credentials/test.ts b/test/apps/credentials/test.ts index 87e557cf8..665f46c5a 100644 --- a/test/apps/credentials/test.ts +++ b/test/apps/credentials/test.ts @@ -17,11 +17,7 @@ describe('credentials', function() { // hooks before(async () => { - // TODO this is brittle. Make it unnecessary - process.chdir(__dirname); - process.env.NODE_ENV = 'production'; - - await build(); + await build({ cwd: __dirname }); runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); ({ base, page, start, prefetchRoutes } = await runner.start()); diff --git a/test/apps/encoding/test.ts b/test/apps/encoding/test.ts index 08618ca23..666643014 100644 --- a/test/apps/encoding/test.ts +++ b/test/apps/encoding/test.ts @@ -17,11 +17,7 @@ describe('encoding', function() { // hooks before(async () => { - // TODO this is brittle. Make it unnecessary - process.chdir(__dirname); - process.env.NODE_ENV = 'production'; - - await build(); + await build({ cwd: __dirname }); runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); ({ base, page, start, prefetchRoutes } = await runner.start()); diff --git a/test/apps/errors/test.ts b/test/apps/errors/test.ts index 1c72ba1e8..57a7dbcc4 100644 --- a/test/apps/errors/test.ts +++ b/test/apps/errors/test.ts @@ -17,11 +17,7 @@ describe('errors', function() { // hooks before(async () => { - // TODO this is brittle. Make it unnecessary - process.chdir(__dirname); - process.env.NODE_ENV = 'production'; - - await build(); + await build({ cwd: __dirname }); runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); ({ base, page, start, prefetchRoutes } = await runner.start()); diff --git a/test/apps/export/test.ts b/test/apps/export/test.ts index 258cb0320..9bb833983 100644 --- a/test/apps/export/test.ts +++ b/test/apps/export/test.ts @@ -7,16 +7,12 @@ describe('export', function() { // hooks before(async () => { - // TODO this is brittle. Make it unnecessary - process.chdir(__dirname); - process.env.NODE_ENV = 'production'; - - await api.build(); - await api.export(); + await api.build({ cwd: __dirname }); + await api.export({ cwd: __dirname }); }); it('crawls a site', () => { - const files = walk('__sapper__/export'); + const files = walk(`${__dirname}/__sapper__/export`); const client_assets = files.filter(file => file.startsWith('client/')); const non_client_assets = files.filter(file => !file.startsWith('client/')).sort(); diff --git a/test/apps/ignore/test.ts b/test/apps/ignore/test.ts index 95c13bd0e..6ec861608 100644 --- a/test/apps/ignore/test.ts +++ b/test/apps/ignore/test.ts @@ -12,11 +12,7 @@ describe('ignore', function() { // hooks before(async () => { - // TODO this is brittle. Make it unnecessary - process.chdir(__dirname); - process.env.NODE_ENV = 'production'; - - await build(); + await build({ cwd: __dirname }); runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); ({ base, page } = await runner.start()); diff --git a/test/apps/layout/test.ts b/test/apps/layout/test.ts index a7d72a96c..1cadbe17a 100644 --- a/test/apps/layout/test.ts +++ b/test/apps/layout/test.ts @@ -16,11 +16,7 @@ describe('layout', function() { // hooks before(async () => { - // TODO this is brittle. Make it unnecessary - process.chdir(__dirname); - process.env.NODE_ENV = 'production'; - - await build(); + await build({ cwd: __dirname }); runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); ({ base, page, start } = await runner.start()); diff --git a/test/apps/preloading/test.ts b/test/apps/preloading/test.ts index 7e321bb66..dccd667c3 100644 --- a/test/apps/preloading/test.ts +++ b/test/apps/preloading/test.ts @@ -20,11 +20,7 @@ describe('preloading', function() { // hooks before(async () => { - // TODO this is brittle. Make it unnecessary - process.chdir(__dirname); - process.env.NODE_ENV = 'production'; - - await build(); + await build({ cwd: __dirname }); runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); ({ base, page, start, prefetchRoutes } = await runner.start()); diff --git a/test/apps/redirects/test.ts b/test/apps/redirects/test.ts index 930be5fed..8054924b7 100644 --- a/test/apps/redirects/test.ts +++ b/test/apps/redirects/test.ts @@ -17,11 +17,7 @@ describe('redirects', function() { // hooks before(async () => { - // TODO this is brittle. Make it unnecessary - process.chdir(__dirname); - process.env.NODE_ENV = 'production'; - - await build(); + await build({ cwd: __dirname }); runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); ({ base, page, start, prefetchRoutes } = await runner.start()); diff --git a/test/apps/scroll/test.ts b/test/apps/scroll/test.ts index fbfee563f..0c0db39b0 100644 --- a/test/apps/scroll/test.ts +++ b/test/apps/scroll/test.ts @@ -17,11 +17,7 @@ describe('scroll', function() { // hooks before(async () => { - // TODO this is brittle. Make it unnecessary - process.chdir(__dirname); - process.env.NODE_ENV = 'production'; - - await build(); + await build({ cwd: __dirname }); runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); ({ base, page, start, prefetchRoutes } = await runner.start()); diff --git a/test/apps/store/test.ts b/test/apps/store/test.ts index d437628c8..a8eab1c10 100644 --- a/test/apps/store/test.ts +++ b/test/apps/store/test.ts @@ -15,11 +15,7 @@ describe('store', function() { // hooks before(async () => { - // TODO this is brittle. Make it unnecessary - process.chdir(__dirname); - process.env.NODE_ENV = 'production'; - - await build(); + await build({ cwd: __dirname }); runner = new AppRunner(__dirname, '__sapper__/build/server/server.js'); ({ base, page, start } = await runner.start()); diff --git a/test/apps/with-basepath/test.ts b/test/apps/with-basepath/test.ts index 1284f0275..f535b03bd 100644 --- a/test/apps/with-basepath/test.ts +++ b/test/apps/with-basepath/test.ts @@ -13,13 +13,10 @@ describe('with-basepath', function() { // hooks before(async () => { - // TODO this is brittle. Make it unnecessary - process.chdir(__dirname); - process.env.NODE_ENV = 'production'; - - await api.build(); + await api.build({ cwd: __dirname }); await api.export({ + cwd: __dirname, basepath: 'custom-basepath' }); @@ -49,7 +46,7 @@ describe('with-basepath', function() { }); it('crawls an exported site with basepath', () => { - const files = walk('__sapper__/export'); + const files = walk(`${__dirname}/__sapper__/export`); const client_assets = files.filter(file => file.startsWith('custom-basepath/client/')); const non_client_assets = files.filter(file => !file.startsWith('custom-basepath/client/')).sort(); From b6a2a72cc7c324977507aa83361ea08e80ea4a4f Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 8 Oct 2018 13:34:14 -0400 Subject: [PATCH 5/9] remove sapper start and sapper upgrade --- src/cli/build.ts | 4 ++-- src/cli/upgrade.ts | 53 ---------------------------------------------- 2 files changed, 2 insertions(+), 55 deletions(-) delete mode 100644 src/cli/upgrade.ts diff --git a/src/cli/build.ts b/src/cli/build.ts index e95c5d535..3d20f620b 100644 --- a/src/cli/build.ts +++ b/src/cli/build.ts @@ -3,7 +3,7 @@ import { build as _build } from '../api/build'; import colors from 'kleur'; import { repeat } from '../utils'; -export function build(opts: { bundler?: 'rollup' | 'webpack', legacy?: boolean }) { +export function build(opts: { bundler?: 'rollup' | 'webpack', legacy?: boolean, dest: string }) { const cwd = path.resolve(process.env.SAPPER_BASE || ''); return _build({ @@ -29,6 +29,6 @@ export function build(opts: { bundler?: 'rollup' | 'webpack', legacy?: boolean } cwd, src: path.resolve(cwd, process.env.SAPPER_SRC || 'src'), routes: path.resolve(cwd, process.env.SAPPER_ROUTES || 'src/routes'), - dest: path.resolve(cwd, process.env.SAPPER_DEST || '__sapper__/build') + dest: path.resolve(cwd, opts.dest) }); } \ No newline at end of file diff --git a/src/cli/upgrade.ts b/src/cli/upgrade.ts deleted file mode 100644 index 7d8b16b57..000000000 --- a/src/cli/upgrade.ts +++ /dev/null @@ -1,53 +0,0 @@ -import * as fs from 'fs'; -import colors from 'kleur'; - -export default async function upgrade() { - const upgraded = [ - await upgrade_sapper_main() - ].filter(Boolean); - - if (upgraded.length === 0) { - console.log(`No changes!`); - } -} - -async function upgrade_sapper_main() { - const _2xx = read('templates/2xx.html'); - const _4xx = read('templates/4xx.html'); - const _5xx = read('templates/5xx.html'); - - const pattern = /