Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

V2: Inline process.browser for better code elimination #3688

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
96 changes: 94 additions & 2 deletions packages/core/integration-tests/test/javascript.js
Expand Up @@ -1005,9 +1005,29 @@ describe('javascript', function() {
assert.equal(output(), 'test:test');
});

it.skip('should not insert environment variables in electron environment', async function() {
it('should not insert environment variables in electron-main environment', async function() {
let b = await bundle(path.join(__dirname, '/integration/env/index.js'), {
target: 'electron',
targets: {
main: {
context: 'electron-main',
distDir: path.join(__dirname, '/integration/env/dist.js'),
},
},
});

let output = await run(b);
assert.ok(output.toString().indexOf('process.env') > -1);
assert.equal(output(), 'test:test');
});

it('should not insert environment variables in electron-renderer environment', async function() {
let b = await bundle(path.join(__dirname, '/integration/env/index.js'), {
targets: {
main: {
context: 'electron-renderer',
distDir: path.join(__dirname, '/integration/env/dist.js'),
},
},
});

let output = await run(b);
Expand Down Expand Up @@ -1057,6 +1077,78 @@ describe('javascript', function() {
assert.equal(output, 'productiontest');
});

it('should replace process.browser for target browser', async function() {
let b = await bundle(
path.join(__dirname, '/integration/process/index.js'),
{
targets: {
main: {
context: 'browser',
distDir: path.join(__dirname, '/integration/process/dist.js'),
},
},
},
);

let output = await run(b);
assert.ok(output.toString().indexOf('process.browser') === -1);
assert.equal(output(), true);
});

it('should not touch process.browser for target node', async function() {
let b = await bundle(
path.join(__dirname, '/integration/process/index.js'),
{
targets: {
main: {
context: 'node',
distDir: path.join(__dirname, '/integration/process/dist.js'),
},
},
},
);

let output = await run(b);
assert.ok(output.toString().indexOf('process.browser') !== -1);
assert.equal(output(), false);
});

it('should not touch process.browser for target electron-main', async function() {
let b = await bundle(
path.join(__dirname, '/integration/process/index.js'),
{
targets: {
main: {
context: 'electron-main',
distDir: path.join(__dirname, '/integration/process/dist.js'),
},
},
},
);

let output = await run(b);
assert.ok(output.toString().indexOf('process.browser') !== -1);
assert.equal(output(), false);
});

it('should replace process.browser for target electron-renderer', async function() {
let b = await bundle(
path.join(__dirname, '/integration/process/index.js'),
{
targets: {
main: {
context: 'electron-renderer',
distDir: path.join(__dirname, '/integration/process/dist.js'),
},
},
},
);

let output = await run(b);
assert.ok(output.toString().indexOf('process.browser') === -1);
assert.equal(output(), true);
});

it.skip('should support adding implicit dependencies', async function() {
let b = await bundle(path.join(__dirname, '/integration/json/index.js'), {
delegate: {
Expand Down
3 changes: 2 additions & 1 deletion packages/core/test-utils/src/utils.js
Expand Up @@ -175,9 +175,10 @@ export async function run(
break;
}
case 'node':
case 'electron-main':
ctx = prepareNodeContext(nullthrows(bundle.filePath), globals);
break;
case 'electron': {
case 'electron-renderer': {
let browser = prepareBrowserContext(nullthrows(bundle.filePath), globals);
ctx = {
...browser.ctx,
Expand Down
18 changes: 14 additions & 4 deletions packages/transformers/js/src/JSTransformer.js
Expand Up @@ -4,7 +4,7 @@ import semver from 'semver';
import generate from '@babel/generator';
import {Transformer} from '@parcel/plugin';
import collectDependencies from './visitors/dependencies';
import envVisitor from './visitors/env';
import processVisitor from './visitors/process';
import fsVisitor from './visitors/fs';
import insertGlobals from './visitors/globals';
import {parse} from '@babel/parser';
Expand All @@ -17,6 +17,7 @@ import SourceMap from '@parcel/source-map';

const IMPORT_RE = /\b(?:import\b|export\b|require\s*\()/;
const ENV_RE = /\b(?:process\.env)\b/;
const BROWSER_RE = /\b(?:process\.browser)\b/;
const GLOBAL_RE = /\b(?:process|__dirname|__filename|global|Buffer|define)\b/;
const FS_RE = /\breadFileSync\b/;
const SW_RE = /\bnavigator\s*\.\s*serviceWorker\s*\.\s*register\s*\(/;
Expand Down Expand Up @@ -46,6 +47,7 @@ export default new Transformer({
!options.scopeHoist &&
!canHaveDependencies(code) &&
!ENV_RE.test(code) &&
!BROWSER_RE.test(code) &&
!FS_RE.test(code)
) {
return null;
Expand Down Expand Up @@ -74,9 +76,17 @@ export default new Transformer({
let ast = asset.ast;
let code = await asset.getCode();

// Inline environment variables
if (!asset.env.isNode() && (ast.isDirty || ENV_RE.test(code))) {
walk.simple(ast.program, envVisitor, {asset, env: options.env});
// Inline process/ environment variables
if (
(!asset.env.isNode() && (ast.isDirty || ENV_RE.test(code))) ||
(asset.env.isBrowser() && (ast.isDirty || BROWSER_RE.test(code)))
) {
walk.ancestor(ast.program, processVisitor, {
mischnic marked this conversation as resolved.
Show resolved Hide resolved
asset,
env: options.env,
isNode: asset.env.isNode(),
isBrowser: asset.env.isBrowser(),
});
}

// Collect dependencies
Expand Down
Expand Up @@ -2,9 +2,9 @@ import * as types from '@babel/types';
import {morph} from './utils';

export default {
MemberExpression(node, {asset, env}) {
MemberExpression(node, {asset, env, isBrowser, isNode}, ancestors) {
// Inline environment variables accessed on process.env
if (types.matchesPattern(node.object, 'process.env')) {
if (!isNode && types.matchesPattern(node.object, 'process.env')) {
let key = types.toComputedKey(node);
if (types.isStringLiteral(key)) {
// Try using the value from the passed env (either from new Parcel
Expand All @@ -17,6 +17,19 @@ export default {
// asset.meta.env[key.value] = process.env[key.value];
}
}
// Inline process.browser
} else if (isBrowser && types.matchesPattern(node, 'process.browser')) {
// the last ancestor is the node itself, the one before may be it's parent
const parent = ancestors[ancestors.length - 2];
const isAssignmentExpression = Boolean(
parent && types.isAssignmentExpression(parent) && parent.left === node,
);

if (isAssignmentExpression) {
parent.right = types.booleanLiteral(true);
} else {
morph(node, types.booleanLiteral(true));
}
}
},
};