Skip to content

Commit

Permalink
feat: Elide comments in bundles (#2420)
Browse files Browse the repository at this point in the history
Closes: #2413 

## Description

Adds an option to the bundler to blank the interior of comments,
reducing bundle sizes.

This change does not attempt to apply the elideComments behavior if the
user selects noTransforms, since it is piggybacking on the censorship
evasion transform. These features could be decoupled, elideComments
works with endoZipBase64 and a narrower interpretation of noTransforms
(no precompiled module transforms).

### Security Considerations

Some care has been taken to ensure that the resulting comments produce
programs with the same behavior in the event the comment must be
interpreted as a newline for automatic semicolon insertion (ASI).

### Scaling Considerations

None.

### Documentation Considerations

- NEWS
- README

### Testing Considerations

- evasive transform unit tests
- bundle source unit tests
  - covering endoScript and endoZipBase64
  - composition errors with noTransforms

Uncovered:
- cache behavior. The new flag participates in the cache and gracefully
handles caches from prior versions.
- command line 

### Compatibility Considerations

Maintains support for caches from prior versions.

### Upgrade Considerations

None
  • Loading branch information
kriskowal authored Aug 20, 2024
2 parents 707bd67 + 0a59732 commit 1f2257f
Show file tree
Hide file tree
Showing 16 changed files with 342 additions and 26 deletions.
7 changes: 7 additions & 0 deletions packages/bundle-source/NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
User-visible changes to `@endo/bundle-source`:

# Next version

- Adds support for `--elide-comments` (`-e`) that blanks out the interior of
comments in `endoScript` and `endoZipBase64` formats to reduce bundle size
without compromising cursor position correspondence between source and bundle
code.

# v3.3.0 (2024-07-30)

- Adds support for `--no-transforms` (`-T`) which generates bundles with
Expand Down
15 changes: 15 additions & 0 deletions packages/bundle-source/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,21 @@ entry instead of `main` in `package.json`, if not overridden by explicit
The `development` condition additionally implies that the bundle may import
`devDependencies` from the package containing the entry module.
## Comment Elision
The `--elide-comments (`-e`) flag with `--format` (`-f`) `endoScript` or
`endoZipBase64` (default) causes the bundler to blank out the interior of
comments, without compromising line or column number cursor advancement.
This can reduce bundle size without harming the debug experience any more than
other transforms.

Comment elision preserves `/*! slashasterbang /` comments and JSDoc comments
with `@preserve`, `@copyright`, `@license` pragmas or the Internet Explorer
`@cc_on` pragma.

Comment elision does not strip comments entirely.
The syntax to begin or end comments remains.

## Source maps

With the `moduleFormat` of `endoZipBase64`, the bundler can generate source
Expand Down
19 changes: 16 additions & 3 deletions packages/bundle-source/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const { Fail, quote: q } = assert;
* @property {string} bundleTime ISO format
* @property {number} bundleSize
* @property {boolean} noTransforms
* @property {boolean} elideComments
* @property {ModuleFormat} format
* @property {string[]} conditions
* @property {{ relative: string, absolute: string }} moduleSource
Expand Down Expand Up @@ -52,6 +53,7 @@ export const makeBundleCache = (wr, cwd, readPowers, opts) => {
* @param {Logger} [log]
* @param {object} [options]
* @param {boolean} [options.noTransforms]
* @param {boolean} [options.elideComments]
* @param {string[]} [options.conditions]
* @param {ModuleFormat} [options.format]
*/
Expand All @@ -60,6 +62,7 @@ export const makeBundleCache = (wr, cwd, readPowers, opts) => {

const {
noTransforms = false,
elideComments = false,
format = 'endoZipBase64',
conditions = [],
} = options;
Expand Down Expand Up @@ -93,7 +96,7 @@ export const makeBundleCache = (wr, cwd, readPowers, opts) => {

const bundle = await bundleSource(
rootPath,
{ ...bundleOptions, noTransforms, format, conditions },
{ ...bundleOptions, noTransforms, elideComments, format, conditions },
{
...readPowers,
read: loggedRead,
Expand Down Expand Up @@ -122,6 +125,7 @@ export const makeBundleCache = (wr, cwd, readPowers, opts) => {
}),
),
noTransforms,
elideComments,
format,
conditions,
};
Expand Down Expand Up @@ -152,6 +156,7 @@ export const makeBundleCache = (wr, cwd, readPowers, opts) => {
* @param {BundleMeta} [meta]
* @param {object} [options]
* @param {boolean} [options.noTransforms]
* @param {boolean} [options.elideComments]
* @param {ModuleFormat} [options.format]
* @param {string[]} [options.conditions]
* @returns {Promise<BundleMeta>}
Expand All @@ -166,6 +171,7 @@ export const makeBundleCache = (wr, cwd, readPowers, opts) => {
await null;
const {
noTransforms: expectedNoTransforms = false,
elideComments: expectedElideComments = false,
format: expectedFormat = DEFAULT_MODULE_FORMAT,
conditions: expectedConditions = [],
} = options;
Expand Down Expand Up @@ -194,12 +200,14 @@ export const makeBundleCache = (wr, cwd, readPowers, opts) => {
moduleSource: { absolute: moduleSource },
format = 'endoZipBase64',
noTransforms = false,
elideComments = false,
conditions = [],
} = meta;
conditions.sort();
assert.equal(bundleFileName, toBundleName(targetName));
assert.equal(format, expectedFormat);
assert.equal(noTransforms, expectedNoTransforms);
assert.equal(elideComments, expectedElideComments);
assert.equal(conditions.length, expectedConditions.length);
conditions.forEach((tag, index) => {
assert.equal(tag, expectedConditions[index]);
Expand Down Expand Up @@ -249,6 +257,7 @@ export const makeBundleCache = (wr, cwd, readPowers, opts) => {
* @param {Logger} [log]
* @param {object} [options]
* @param {boolean} [options.noTransforms]
* @param {boolean} [options.elideComments]
* @param {ModuleFormat} [options.format]
* @param {string[]} [options.conditions]
* @returns {Promise<BundleMeta>}
Expand All @@ -269,13 +278,15 @@ export const makeBundleCache = (wr, cwd, readPowers, opts) => {
meta = await validate(targetName, rootPath, log, meta, {
format: options.format,
noTransforms: options.noTransforms,
elideComments: options.elideComments,
conditions: options.conditions,
});
const {
bundleTime,
bundleSize,
contents,
noTransforms,
elideComments,
format = 'endoZipBase64',
conditions = [],
} = meta;
Expand All @@ -288,7 +299,7 @@ export const makeBundleCache = (wr, cwd, readPowers, opts) => {
bundleTime,
'with size',
bundleSize,
noTransforms ? 'w/o transforms' : 'with transforms',
`${noTransforms ? 'w/o transforms' : 'with transforms'}${elideComments ? ' and comments elided' : ''}`,
'with format',
format,
'and conditions',
Expand All @@ -308,6 +319,7 @@ export const makeBundleCache = (wr, cwd, readPowers, opts) => {
bundleTime,
contents,
noTransforms,
elideComments,
format = 'endoZipBase64',
conditions = [],
} = meta;
Expand All @@ -319,7 +331,7 @@ export const makeBundleCache = (wr, cwd, readPowers, opts) => {
bundleFileName,
'at',
bundleTime,
noTransforms ? 'w/o transforms' : 'with transforms',
`${noTransforms ? 'w/o transforms' : 'with transforms'}${elideComments ? ' and comments elided' : ''}`,
'with format',
format,
'and conditions',
Expand All @@ -337,6 +349,7 @@ export const makeBundleCache = (wr, cwd, readPowers, opts) => {
* @param {Logger} [log]
* @param {object} [options]
* @param {boolean} [options.noTransforms]
* @param {boolean} [options.elideComments]
* @param {ModuleFormat} [options.format]
* @param {string[]} [options.conditions]
*/
Expand Down
4 changes: 4 additions & 0 deletions packages/bundle-source/demo/meaningful.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
import json from './meaning.json';

/**
* This comment will be blanked with the elideComments / --elide-comments
* feature enabled.
*/
export const meaning = json.meaning;
9 changes: 8 additions & 1 deletion packages/bundle-source/src/endo.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,14 @@ const textDecoder = new TextDecoder();

export const makeBundlingKit = (
{ pathResolve, userInfo, computeSha512, platform, env },
{ cacheSourceMaps, noTransforms, commonDependencies, dev },
{ cacheSourceMaps, elideComments, noTransforms, commonDependencies, dev },
) => {
if (noTransforms && elideComments) {
throw new Error(
'bundleSource endoZipBase64 cannot elideComments with noTransforms',
);
}

const sourceMapJobs = new Set();
let writeSourceMap = Function.prototype;
if (cacheSourceMaps) {
Expand Down Expand Up @@ -112,6 +118,7 @@ export const makeBundlingKit = (
sourceType: babelSourceType,
sourceMap,
sourceMapUrl: new URL(specifier, location).href,
elideComments,
}));
const objectBytes = textEncoder.encode(object);
return { bytes: objectBytes, parser, sourceMap };
Expand Down
16 changes: 15 additions & 1 deletion packages/bundle-source/src/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,20 @@ bundle-source [-Tft] --cache-js|--cache-json <cache/> (<entry.js> <bundle-name>)
-f,--format endoZipBase64*|nestedEvaluate|getExport
-C,--condition <condition> (browser, node, development, &c)
-C development (to access devDependencies)
-T,--no-transforms`;
-T,--no-transforms
-e,--elide-comments`;

const options = /** @type {const} */ ({
'no-transforms': {
type: 'boolean',
short: 'T',
multiple: false,
},
'elide-comments': {
type: 'boolean',
short: 'e',
multiple: false,
},
'cache-js': {
type: 'string',
multiple: false,
Expand Down Expand Up @@ -61,6 +67,7 @@ export const main = async (args, { loadModule, pid, log }) => {
format: moduleFormat = 'endoZipBase64',
condition: conditions = [],
'no-transforms': noTransforms,
'elide-comments': elideComments,
'cache-json': cacheJson,
'cache-js': cacheJs,
// deprecated
Expand All @@ -69,6 +76,11 @@ export const main = async (args, { loadModule, pid, log }) => {
positionals,
} = parseArgs({ args, options, allowPositionals: true });

if (noTransforms && elideComments) {
throw Error(
`Cannot elide comments without transforms (-T,--no-transforms + -e,--elide-comments)`,
);
}
if (!SUPPORTED_FORMATS.includes(moduleFormat)) {
throw Error(`Unsupported format: ${moduleFormat}\n\n${USAGE}`);
}
Expand All @@ -94,6 +106,7 @@ export const main = async (args, { loadModule, pid, log }) => {
const [entryPath] = positionals;
const bundle = await bundleSource(entryPath, {
noTransforms,
elideComments,
format,
conditions,
});
Expand Down Expand Up @@ -125,6 +138,7 @@ export const main = async (args, { loadModule, pid, log }) => {
// eslint-disable-next-line no-await-in-loop
await cache.validateOrAdd(bundleRoot, bundleName, undefined, {
noTransforms,
elideComments,
format,
conditions,
});
Expand Down
4 changes: 4 additions & 0 deletions packages/bundle-source/src/script.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const readPowers = makeReadPowers({ fs, url, crypto });
* @param {boolean} [options.dev]
* @param {boolean} [options.cacheSourceMaps]
* @param {boolean} [options.noTransforms]
* @param {boolean} [options.elideComments]
* @param {Record<string, string>} [options.commonDependencies]
* @param {object} [grantedPowers]
* @param {(bytes: string | Uint8Array) => string} [grantedPowers.computeSha512]
Expand All @@ -37,9 +38,11 @@ export async function bundleScript(
dev = false,
cacheSourceMaps = false,
noTransforms = false,
elideComments = false,
conditions = [],
commonDependencies,
} = options;

const powers = { ...readPowers, ...grantedPowers };
const {
computeSha512,
Expand All @@ -63,6 +66,7 @@ export async function bundleScript(
{
cacheSourceMaps,
noTransforms,
elideComments,
commonDependencies,
dev,
},
Expand Down
3 changes: 3 additions & 0 deletions packages/bundle-source/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ export {};
* @property {T} [format]
* @property {boolean} [dev] - development mode, for test bundles that need
* access to devDependencies of the entry package.
* @property {boolean} [elideComments] - when true for the `endoScript` and
* `endoZipBase64` format, replaces the interior of comments with blank space
* that advances the cursor the same number of lines and columns.
* @property {boolean} [noTransforms] - when true, generates a bundle with the
* original sources instead of SES-shim specific ESM and CJS. This may become
* default in a future major version.
Expand Down
3 changes: 3 additions & 0 deletions packages/bundle-source/src/zip-base64.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const readPowers = makeReadPowers({ fs, url, crypto });
* @param {boolean} [options.dev]
* @param {boolean} [options.cacheSourceMaps]
* @param {boolean} [options.noTransforms]
* @param {boolean} [options.elideComments]
* @param {string[]} [options.conditions]
* @param {Record<string, string>} [options.commonDependencies]
* @param {object} [grantedPowers]
Expand All @@ -40,6 +41,7 @@ export async function bundleZipBase64(
dev = false,
cacheSourceMaps = false,
noTransforms = false,
elideComments = false,
conditions = [],
commonDependencies,
} = options;
Expand All @@ -66,6 +68,7 @@ export async function bundleZipBase64(
{
cacheSourceMaps,
noTransforms,
elideComments,
commonDependencies,
dev,
},
Expand Down
18 changes: 16 additions & 2 deletions packages/bundle-source/test/endo-script-format.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,30 @@ import test from '@endo/ses-ava/prepare-endo.js';
import * as url from 'url';
import bundleSource from '../src/index.js';

test('endo script format', async t => {
const generate = async (options = {}) => {
const entryPath = url.fileURLToPath(
new URL(`../demo/meaning.js`, import.meta.url),
);
const bundle = await bundleSource(entryPath, {
return bundleSource(entryPath, {
format: 'endoScript',
...options,
});
};

test('endo script format', async t => {
const bundle = await generate();
t.is(bundle.moduleFormat, 'endoScript');
const { source } = bundle;
const compartment = new Compartment();
const ns = compartment.evaluate(source);
t.is(ns.meaning, 42);
});

test('endo script format is smaller with blank comments', async t => {
const bigBundle = await generate();
const smallBundle = await generate({ elideComments: true });
const compartment = new Compartment();
const ns = compartment.evaluate(smallBundle.source);
t.is(ns.meaning, 42);
t.assert(smallBundle.source.length < bigBundle.source.length);
});
6 changes: 6 additions & 0 deletions packages/evasive-transform/NEWS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
User-visible changes in `@endo/evasive-transform`:

# Next release

- Adds an `elideComments` option to replace the interior of comments with
minimal blank space with identical cursor advancement behavior.
17 changes: 14 additions & 3 deletions packages/evasive-transform/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { generate } from './generate.js';
* @property {string|import('source-map').RawSourceMap} [sourceMap] - Original source map in JSON string or object form
* @property {string} [sourceUrl] - URL or filepath of the original source in `code`
* @property {boolean} [useLocationUnmap] - Enable location unmapping. Only applies if `sourceMap` was provided
* @property {boolean} [elideComments] - Replace comments with an ellipsis but preserve interior newlines.
* @property {import('./parse-ast.js').SourceType} [sourceType] - Module source type
* @public
*/
Expand All @@ -41,7 +42,13 @@ import { generate } from './generate.js';
export function evadeCensorSync(source, options) {
// TODO Use options ?? {} when resolved:
// https://github.com/Agoric/agoric-sdk/issues/8671
const { sourceMap, sourceUrl, useLocationUnmap, sourceType } = options || {};
const {
sourceMap,
sourceUrl,
useLocationUnmap,
sourceType,
elideComments = false,
} = options || {};

// Parse the rolled-up chunk with Babel.
// We are prepared for different module systems.
Expand All @@ -53,9 +60,13 @@ export function evadeCensorSync(source, options) {
typeof sourceMap === 'string' ? sourceMap : JSON.stringify(sourceMap);

if (sourceMap && useLocationUnmap) {
transformAst(ast, { sourceMap: sourceMapJson, useLocationUnmap });
transformAst(ast, {
sourceMap: sourceMapJson,
useLocationUnmap,
elideComments,
});
} else {
transformAst(ast);
transformAst(ast, { elideComments });
}

if (sourceUrl) {
Expand Down
Loading

0 comments on commit 1f2257f

Please sign in to comment.