Skip to content

Commit

Permalink
Fix dev server hanging on binary files (#1017)
Browse files Browse the repository at this point in the history
  • Loading branch information
drwpow committed Sep 8, 2020
1 parent baa441f commit 286e10f
Show file tree
Hide file tree
Showing 12 changed files with 67 additions and 52 deletions.
1 change: 1 addition & 0 deletions snowpack/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
"got": "^11.1.4",
"http-proxy": "^1.18.1",
"is-builtin-module": "^3.0.0",
"isbinaryfile": "^4.0.6",
"jsonschema": "^1.2.5",
"kleur": "^4.1.0",
"mime-types": "^2.1.26",
Expand Down
5 changes: 2 additions & 3 deletions snowpack/src/build/build-pipeline.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import {promises as fs} from 'fs';
import path from 'path';
import {validatePluginLoadResult} from '../config';
import {logger} from '../logger';
import {SnowpackBuildMap, SnowpackConfig, SnowpackPlugin} from '../types/snowpack';
import {getEncodingType, getExt, replaceExt} from '../util';
import {getExt, readFile, replaceExt} from '../util';

export interface BuildFileOptions {
isDev: boolean;
Expand Down Expand Up @@ -97,7 +96,7 @@ async function runPipelineLoadStep(

return {
[srcExt]: {
code: await fs.readFile(srcPath, getEncodingType(srcExt)),
code: await readFile(srcPath),
},
};
}
Expand Down
20 changes: 13 additions & 7 deletions snowpack/src/commands/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {removeLeadingSlash} from '../config';
import {logger} from '../logger';
import {transformFileImports} from '../rewrite-imports';
import {CommandOptions, ImportMap, SnowpackConfig, SnowpackSourceFile} from '../types/snowpack';
import {cssSourceMappingURL, getEncodingType, jsSourceMappingURL, replaceExt} from '../util';
import {cssSourceMappingURL, jsSourceMappingURL, readFile, replaceExt} from '../util';
import {getInstallTargets, run as installRunner} from './install';

const CONCURRENT_WORKERS = require('os').cpus().length;
Expand Down Expand Up @@ -160,7 +160,12 @@ class FileBuilder {
async resolveImports(importMap: ImportMap) {
let isSuccess = true;
this.filesToProxy = [];
for (const [outLoc, file] of Object.entries(this.filesToResolve)) {
for (const [outLoc, rawFile] of Object.entries(this.filesToResolve)) {
// don’t transform binary file contents
if (Buffer.isBuffer(rawFile.contents)) {
continue;
}
const file = rawFile as SnowpackSourceFile<string>;
const resolveImportSpecifier = createImportResolver({
fileLoc: file.locOnDisk!, // we’re confident these are reading from disk because we just read them
dependencyImportMap: importMap,
Expand All @@ -174,7 +179,7 @@ class FileBuilder {
// Until supported, just exit here.
if (!resolvedImportUrl) {
isSuccess = false;
logger.error(`${file.locOnDisk} - Could not resolve unkonwn import "${spec}".`);
logger.error(`${file.locOnDisk} - Could not resolve unknown import "${spec}".`);
return spec;
}
// Ignore "http://*" imports
Expand Down Expand Up @@ -227,7 +232,8 @@ class FileBuilder {
async writeToDisk() {
mkdirp.sync(this.outDir);
for (const [outLoc, code] of Object.entries(this.output)) {
await fs.writeFile(outLoc, code, getEncodingType(path.extname(outLoc)));
const encoding = typeof code === 'string' ? 'utf-8' : undefined;
await fs.writeFile(outLoc, code, encoding);
}
}

Expand All @@ -243,7 +249,7 @@ class FileBuilder {
hmr: false,
config: this.config,
});
await fs.writeFile(importProxyFileLoc, proxyCode, getEncodingType('.js'));
await fs.writeFile(importProxyFileLoc, proxyCode, 'utf-8');
}
}

Expand Down Expand Up @@ -317,7 +323,7 @@ export async function command(commandOptions: CommandOptions) {
!installedFileLoc.endsWith('import-map.json') &&
path.extname(installedFileLoc) !== '.js'
) {
const proxiedCode = await fs.readFile(installedFileLoc, {encoding: 'utf-8'});
const proxiedCode = await readFile(installedFileLoc);
const importProxyFileLoc = installedFileLoc + '.proxy.js';
const proxiedUrl = installedFileLoc.substr(buildDirectoryLoc.length).replace(/\\/g, '/');
const proxyCode = await wrapImportProxy({
Expand All @@ -326,7 +332,7 @@ export async function command(commandOptions: CommandOptions) {
hmr: false,
config: config,
});
await fs.writeFile(importProxyFileLoc, proxyCode, getEncodingType('.js'));
await fs.writeFile(importProxyFileLoc, proxyCode, 'utf-8');
}
}
return installResult;
Expand Down
10 changes: 7 additions & 3 deletions snowpack/src/commands/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,11 @@ import {
checkLockfileHash,
cssSourceMappingURL,
DEV_DEPENDENCIES_DIR,
getEncodingType,
getExt,
jsSourceMappingURL,
openInBrowser,
parsePackageImportSpecifier,
readFile,
replaceExt,
resolveDependencyManifest,
updateLockfileHash,
Expand Down Expand Up @@ -166,7 +166,7 @@ const sendFile = (
}

res.writeHead(200, headers);
res.write(body, getEncodingType(ext) as BufferEncoding);
res.write(body);
res.end();
};

Expand Down Expand Up @@ -242,6 +242,7 @@ export async function command(commandOptions: CommandOptions) {

// Set the proper install options, in case an install is needed.
const dependencyImportMapLoc = path.join(DEV_DEPENDENCIES_DIR, 'import-map.json');
logger.debug(`Using cache folder: ${path.relative(cwd, DEV_DEPENDENCIES_DIR)}`);
const installCommandOptions = merge(commandOptions, {
config: {
installOptions: {
Expand All @@ -254,9 +255,12 @@ export async function command(commandOptions: CommandOptions) {

// Start with a fresh install of your dependencies, if needed.
if (!(await checkLockfileHash(DEV_DEPENDENCIES_DIR)) || !existsSync(dependencyImportMapLoc)) {
logger.debug('Cache out of date or missing. Updating…');
logger.info(colors.yellow('! updating dependencies...'));
await installCommand(installCommandOptions);
await updateLockfileHash(DEV_DEPENDENCIES_DIR);
} else {
logger.debug(`Cache up-to-date. Using existing cache`);
}

let dependencyImportMap: ImportMap = {imports: {}};
Expand Down Expand Up @@ -678,7 +682,7 @@ If Snowpack is having trouble detecting the import, add ${colors.bold(
}

// 2. Load the file from disk. We'll need it to check the cold cache or build from scratch.
const fileContents = await fs.readFile(fileLoc, getEncodingType(requestedFileExt));
const fileContents = await readFile(fileLoc);

// 3. Send dependencies directly, since they were already build & resolved at install time.
if (reqPath.startsWith(config.buildOptions.webModulesUrl) && !isProxyModule) {
Expand Down
6 changes: 6 additions & 0 deletions snowpack/src/commands/install.ts
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,7 @@ export async function getInstallTargets(
if (webDependencies) {
installTargets.push(...scanDepList(Object.keys(webDependencies), cwd));
}
// TODO: remove this if block; move logic inside scanImports
if (scannedFiles) {
installTargets.push(...(await scanImportsFromFiles(scannedFiles, config)));
} else {
Expand All @@ -457,14 +458,19 @@ export async function getInstallTargets(
export async function command(commandOptions: CommandOptions) {
const {cwd, config} = commandOptions;

logger.debug('Starting install');
const installTargets = await getInstallTargets(config);
logger.debug('Received install targets');
if (installTargets.length === 0) {
logger.error('Nothing to install.');
return;
}
logger.debug('Running install command');
const finalResult = await run({...commandOptions, installTargets});
logger.debug('Install command successfully ran');
if (finalResult.newLockfile) {
await writeLockfile(path.join(cwd, 'snowpack.lock.json'), finalResult.newLockfile);
logger.debug('Successfully wrote lockfile');
}
if (finalResult.stats) {
logger.info(printStats(finalResult.stats));
Expand Down
4 changes: 2 additions & 2 deletions snowpack/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import buildScriptPlugin from '@snowpack/plugin-build-script';
import runScriptPlugin from '@snowpack/plugin-run-script';
import {cosmiconfigSync} from 'cosmiconfig';
import {all as merge} from 'deepmerge';
import fs from 'fs';
import http from 'http';
import {validate, ValidatorResult} from 'jsonschema';
import path from 'path';
Expand All @@ -22,6 +21,7 @@ import {
LegacySnowpackPlugin,
PluginLoadResult,
} from './types/snowpack';
import {readFile} from './util';

const CONFIG_NAME = 'snowpack';
const ALWAYS_EXCLUDE = ['**/node_modules/**/*', '**/.types/**/*'];
Expand Down Expand Up @@ -267,7 +267,7 @@ function loadPlugins(
plugin.load = async (options: PluginLoadOptions) => {
const result = await build({
...options,
contents: fs.readFileSync(options.filePath, 'utf-8'),
contents: await readFile(options.filePath),
}).catch((err) => {
logger.error(
`[${plugin.name}] There was a problem running this older plugin. Please update the plugin to the latest version.`,
Expand Down
2 changes: 1 addition & 1 deletion snowpack/src/rewrite-imports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ async function transformCssImports(code: string, replaceImport: (specifier: stri
}

export async function transformFileImports(
{baseExt, contents}: SnowpackSourceFile,
{baseExt, contents}: SnowpackSourceFile<string>,
replaceImport: (specifier: string) => string,
) {
if (baseExt === '.js') {
Expand Down
50 changes: 21 additions & 29 deletions snowpack/src/scan-imports.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
import {ImportSpecifier, init as initESModuleLexer, parse} from 'es-module-lexer';
import fs from 'fs';
import glob from 'glob';
import * as colors from 'kleur/colors';
import mime from 'mime-types';
import path from 'path';
import stripComments from 'strip-comments';
import srcFileExtensionMapping from './build/src-file-extension-mapping';
import {logger} from './logger';
import {InstallTarget, SnowpackConfig, SnowpackSourceFile} from './types/snowpack';
import {
Expand All @@ -14,6 +10,7 @@ import {
getExt,
HTML_JS_REGEX,
isTruthy,
readFile,
SVELTE_VUE_REGEX,
} from './util';

Expand Down Expand Up @@ -155,23 +152,38 @@ function parseFileForInstallTargets({
locOnDisk,
baseExt,
contents,
}: SnowpackSourceFile): InstallTarget[] {
}: SnowpackSourceFile<string>): InstallTarget[] {
const relativeLoc = path.relative(process.cwd(), locOnDisk);

try {
switch (baseExt) {
case '.css':
case '.less':
case '.sass':
case '.scss': {
logger.debug(`Scanning ${relativeLoc} for imports as CSS`);
return parseCssForInstallTargets(contents);
}
case '.html':
case '.svelte':
case '.vue': {
logger.debug(`Scanning ${relativeLoc} for imports as HTML`);
return parseJsForInstallTargets(extractJSFromHTML({contents, baseExt}));
}
default: {
case '.js':
case '.jsx':
case '.mjs':
case '.ts':
case '.tsx': {
logger.debug(`Scanning ${relativeLoc} for imports as JS`);
return parseJsForInstallTargets(contents);
}
default: {
logger.debug(
`Skip scanning ${relativeLoc} for imports (unknown file extension ${baseExt})`,
);
return [];
}
}
} catch (err) {
// Another error! No hope left, just abort.
Expand Down Expand Up @@ -235,32 +247,11 @@ export async function scanImports(cwd: string, config: SnowpackConfig): Promise<
const loadedFiles: (SnowpackSourceFile | null)[] = await Promise.all(
includeFiles.map(async (filePath) => {
const {baseExt, expandedExt} = getExt(filePath);
// Always ignore dotfiles
if (filePath.startsWith('.')) {
return null;
}

// Probably a license, a README, etc
if (baseExt === '') {
return null;
}

// If we don't recognize the file type, it could be source. Warn just in case.
if (!Object.keys(srcFileExtensionMapping).includes(baseExt) && !mime.lookup(baseExt)) {
logger.warn(
colors.dim(
`ignoring unsupported file "${path
.relative(process.cwd(), filePath)
.replace(/\\/g, '/')}"`,
),
);
}

return {
baseExt,
expandedExt,
locOnDisk: filePath,
contents: await fs.promises.readFile(filePath, 'utf-8'),
contents: await readFile(filePath),
};
}),
);
Expand All @@ -273,7 +264,8 @@ export async function scanImportsFromFiles(
config: SnowpackConfig,
): Promise<InstallTarget[]> {
return loadedFiles
.map(parseFileForInstallTargets)
.filter((sourceFile) => !Buffer.isBuffer(sourceFile.contents)) // filter out binary files from import scanning
.map((sourceFile) => parseFileForInstallTargets(sourceFile as SnowpackSourceFile<string>))
.reduce((flat, item) => flat.concat(item), [])
.filter((target) => {
const aliasEntry = findMatchingAliasEntry(config, target.specifier);
Expand Down
6 changes: 3 additions & 3 deletions snowpack/src/types/snowpack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ export type SnowpackBuiltFile = {code: string | Buffer; map?: string};
export type SnowpackBuildMap = Record<string, SnowpackBuiltFile>;

/** Standard file interface */
export interface SnowpackSourceFile {
export interface SnowpackSourceFile<Type = string | Buffer> {
/** base extension (e.g. `.js`) */
baseExt: string;
/** file contents */
contents: string;
contents: Type;
/** expanded extension (e.g. `.proxy.js` or `.module.css`) */
expandedExt: string;
/** if no location on disk, assume this exists in memory */
Expand Down Expand Up @@ -81,7 +81,7 @@ export interface SnowpackPlugin {

export interface LegacySnowpackPlugin {
defaultBuildScript: string;
build?(options: PluginLoadOptions & {contents: string}): Promise<any>;
build?(options: PluginLoadOptions & {contents: string | Buffer}): Promise<any>;
bundle?(options: {
srcDirectory: string;
destDirectory: string;
Expand Down
9 changes: 6 additions & 3 deletions snowpack/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import projectCacheDir from 'find-cache-dir';
import findUp from 'find-up';
import fs from 'fs';
import got, {CancelableRequest, Response} from 'got';
import {isBinaryFile} from 'isbinaryfile';
import mkdirp from 'mkdirp';
import open from 'open';
import path from 'path';
Expand Down Expand Up @@ -37,9 +38,11 @@ export const SVELTE_VUE_REGEX = /(<script[^>]*>)(.*?)<\/script>/gims;

export const URL_HAS_PROTOCOL_REGEX = /^(\w+:)?\/\//;

const UTF8_FORMATS = ['.css', '.html', '.js', '.map', '.mjs', '.json', '.svg', '.txt', '.xml'];
export function getEncodingType(ext: string): 'utf-8' | undefined {
return UTF8_FORMATS.includes(ext) ? 'utf-8' : undefined;
/** Read file from disk; return a string if it’s a code file */
export async function readFile(filepath: string): Promise<string | Buffer> {
const data = await fs.promises.readFile(filepath);
const isBinary = await isBinaryFile(data);
return isBinary ? data : data.toString('utf-8');
}

export async function readLockfile(cwd: string): Promise<ImportMap | null> {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
[snowpack] ignoring unsupported file "src/c.abcdefg"
[snowpack] Nothing to install.
5 changes: 5 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -8335,6 +8335,11 @@ isarray@^2.0.1:
resolved "https://registry.yarnpkg.com/isarray/-/isarray-2.0.5.tgz#8af1e4c1221244cc62459faf38940d4e644a5723"
integrity sha512-xHjhDr3cNBK0BzdUJSPXZntQUx/mwMS5Rw4A7lPJ90XGAO6ISP/ePDNuo0vhqOZU+UD5JoodwCAAoZQd3FeAKw==

isbinaryfile@^4.0.6:
version "4.0.6"
resolved "https://registry.yarnpkg.com/isbinaryfile/-/isbinaryfile-4.0.6.tgz#edcb62b224e2b4710830b67498c8e4e5a4d2610b"
integrity sha512-ORrEy+SNVqUhrCaal4hA4fBzhggQQ+BaLntyPOdoEiwlKZW9BZiJXjg3RMiruE4tPEI3pyVPpySHQF/dKWperg==

isexe@^2.0.0:
version "2.0.0"
resolved "https://registry.yarnpkg.com/isexe/-/isexe-2.0.0.tgz#e8fbf374dc556ff8947a10dcb0572d633f2cfa10"
Expand Down

0 comments on commit 286e10f

Please sign in to comment.