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

feat(@parcel/transformer-js): Rewrite __dirname/__filename to be relative to asset.filePath #7727

Merged
merged 50 commits into from
Apr 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
2c3b500
initial
LekoArts Feb 17, 2022
c24fccb
revert some changes
LekoArts Feb 22, 2022
a7e7b3a
add path import
LekoArts Feb 22, 2022
eca555e
test wip
LekoArts Feb 22, 2022
f3a29cb
Merge branch 'v2' into dirname-relative-path
LekoArts Mar 14, 2022
b7b49e3
re-add example
LekoArts Mar 14, 2022
8d73fe0
update swc
LekoArts Mar 15, 2022
2a45008
update example
LekoArts Mar 18, 2022
08bd388
move logic to new replacer
LekoArts Mar 18, 2022
a2d6f12
edit packager
LekoArts Mar 18, 2022
fd1a569
Merge branch 'v2' into dirname-relative-path
LekoArts Mar 18, 2022
a231c46
delete example once more
LekoArts Mar 22, 2022
1646589
revert a change
LekoArts Mar 22, 2022
87df079
add js transformer test
LekoArts Mar 22, 2022
e8e9a4d
fix filename
LekoArts Mar 22, 2022
a634a00
revert typo
LekoArts Mar 22, 2022
9da76e9
apply first batch of review comments
LekoArts Mar 23, 2022
1d820d5
test changes
LekoArts Mar 23, 2022
84b9301
add comment
LekoArts Mar 23, 2022
6251b3d
Merge branch 'v2' into dirname-relative-path
LekoArts Mar 23, 2022
ef6cebf
trigger ci?
LekoArts Mar 23, 2022
e8c6f4d
test changes
LekoArts Mar 24, 2022
95c093f
correct pathing in all os?
LekoArts Mar 28, 2022
748eac3
Merge branch 'v2' into dirname-relative-path
mischnic Mar 28, 2022
1b186df
pray to the testing gods
LekoArts Mar 29, 2022
ee9faaf
Merge branch 'v2' into dirname-relative-path
LekoArts Mar 29, 2022
2c7a4c8
use path util
LekoArts Mar 29, 2022
e5d78e0
fix lint error
LekoArts Mar 29, 2022
b3a92ab
trigger ci
LekoArts Mar 29, 2022
ffac3ec
trigger ci
LekoArts Mar 29, 2022
1f0ebae
replace assets individually
LekoArts Mar 30, 2022
f5e8337
Merge branch 'v2' into dirname-relative-path
LekoArts Mar 30, 2022
3cba5c2
update rust code
LekoArts Mar 30, 2022
ad46e2e
trigger ci
LekoArts Mar 31, 2022
5286520
trigger ci
LekoArts Mar 31, 2022
d249b88
add asset.meta.has_node_replacements
LekoArts Mar 31, 2022
f351f21
flow is weird 🙈
LekoArts Mar 31, 2022
1d1b927
trigger ci
LekoArts Mar 31, 2022
8cabe10
Merge branch 'v2' into dirname-relative-path
LekoArts Mar 31, 2022
762b3c8
trigger ci
LekoArts Apr 1, 2022
256b7c2
Merge branch 'v2' into dirname-relative-path
LekoArts Apr 6, 2022
997e407
use filename OsStr instead of pathdiff in __filename
LekoArts Apr 7, 2022
151a54e
trigger ci
LekoArts Apr 7, 2022
0b45828
Merge branch 'v2' into dirname-relative-path
mischnic Apr 7, 2022
7d313ca
use simpler fold_with
LekoArts Apr 7, 2022
b415560
move replace call up in scopeHoistingPackager
LekoArts Apr 7, 2022
0d96930
trigger ci
LekoArts Apr 7, 2022
c7bc555
Merge branch 'v2' into dirname-relative-path
mischnic Apr 7, 2022
ae7ce09
Merge branch 'v2' into dirname-relative-path
devongovett Apr 11, 2022
43e16a9
Merge branch 'v2' into dirname-relative-path
mischnic Apr 11, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion packages/core/integration-tests/test/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,10 @@ describe('fs', function () {
assert(contents.includes("require('fs')"));
assert(contents.includes('readFileSync'));

await outputFS.writeFile(path.join(distDir, 'test.txt'), 'hey');
await outputFS.writeFile(
path.join(__dirname, '/integration/fs-node/', 'test.txt'),
'hey',
);
let output = await run(b);
assert.equal(output, 'hey');
});
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
const fs = require('fs');
const path = require('path');
const otherFunction = require('./other/function')
const subFunction = require('./sub/index')

module.exports = function () {
const data = fs.readFileSync(path.join(__dirname, 'data', 'test.txt'), 'utf8')
const firstDirnameTest = path.join(__dirname, 'data')
const secondDirnameTest = path.join(__dirname, 'other-data')
const firstFilenameTest = __filename
const secondFilenameTest = `${__filename}?query-string=test`
const other = otherFunction()
const sub = subFunction()

return {
data,
firstDirnameTest,
secondDirnameTest,
firstFilenameTest,
secondFilenameTest,
other,
sub,
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
const fs = require('fs');
const path = require('path');

module.exports = function () {
return fs.readFileSync(path.join(__dirname, '..', 'data', 'test.txt'), 'utf8')
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "env-node-replacements",
"engines": {
"node": ">=14"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module.exports = function () {
return {
dirname: __dirname,
filename: __filename
}
}
135 changes: 135 additions & 0 deletions packages/core/integration-tests/test/javascript.js
Original file line number Diff line number Diff line change
Expand Up @@ -2678,6 +2678,141 @@ describe('javascript', function () {
});
});

it('should replace __dirname and __filename with path relative to asset.filePath', async function () {
let b = await bundle(
path.join(__dirname, '/integration/env-node-replacements/index.js'),
);

let dist = await outputFS.readFile(b.getBundles()[0].filePath, 'utf8');
assert(
dist.includes(
'require("path").resolve(__dirname, "../test/integration/env-node-replacements")',
),
);
assert(
dist.includes(
'require("path").resolve(__dirname, "../test/integration/env-node-replacements/other")',
),
);
assert(
dist.includes(
'require("path").resolve(__dirname, "../test/integration/env-node-replacements", "index.js")',
),
);
assert(
dist.includes(
'require("path").resolve(__dirname, "../test/integration/env-node-replacements/sub")',
),
);
assert(
dist.includes(
'require("path").resolve(__dirname, "../test/integration/env-node-replacements/sub", "index.js")',
),
);
let f = await run(b);
let output = f();
assert.equal(output.data, 'hello');
assert.equal(output.other, 'hello');
assert.equal(
output.firstDirnameTest,
path.join(__dirname, '/integration/env-node-replacements/data'),
);
assert.equal(
output.secondDirnameTest,
path.join(__dirname, '/integration/env-node-replacements/other-data'),
);
assert.equal(
output.firstFilenameTest,
path.join(__dirname, '/integration/env-node-replacements/index.js'),
);
assert.equal(
output.secondFilenameTest,
path.join(
__dirname,
'/integration/env-node-replacements/index.js?query-string=test',
),
);
assert.equal(
output.sub.dirname,
path.join(__dirname, '/integration/env-node-replacements/sub'),
);
assert.equal(
output.sub.filename,
path.join(__dirname, '/integration/env-node-replacements/sub/index.js'),
);
});

it('should replace __dirname and __filename with path relative to asset.filePath with scope hoisting', async function () {
let b = await bundle(
path.join(__dirname, '/integration/env-node-replacements/index.js'),
{
mode: 'production',
defaultTargetOptions: {
shouldScopeHoist: true,
shouldOptimize: false,
},
},
);

let dist = await outputFS.readFile(b.getBundles()[0].filePath, 'utf8');
assert(
dist.includes(
'path.resolve(__dirname, "../test/integration/env-node-replacements")',
),
);
assert(
dist.includes(
'path.resolve(__dirname, "../test/integration/env-node-replacements/other")',
),
);
assert(
dist.includes(
'path.resolve(__dirname, "../test/integration/env-node-replacements", "index.js")',
),
);
assert(
dist.includes(
'path.resolve(__dirname, "../test/integration/env-node-replacements/sub")',
),
);
assert(
dist.includes(
'path.resolve(__dirname, "../test/integration/env-node-replacements/sub", "index.js")',
),
);
let f = await run(b);
let output = f();
assert.equal(output.data, 'hello');
assert.equal(output.other, 'hello');
assert.equal(
output.firstDirnameTest,
path.join(__dirname, '/integration/env-node-replacements/data'),
);
assert.equal(
output.secondDirnameTest,
path.join(__dirname, '/integration/env-node-replacements/other-data'),
);
assert.equal(
output.firstFilenameTest,
path.join(__dirname, '/integration/env-node-replacements/index.js'),
);
assert.equal(
output.secondFilenameTest,
path.join(
__dirname,
'/integration/env-node-replacements/index.js?query-string=test',
),
);
assert.equal(
output.sub.dirname,
path.join(__dirname, '/integration/env-node-replacements/sub'),
);
assert.equal(
output.sub.filename,
path.join(__dirname, '/integration/env-node-replacements/sub/index.js'),
);
});

it('should work when multiple files use globals with scope hoisting', async function () {
let b = await bundle(
path.join(__dirname, '/integration/globals/multiple.js'),
Expand Down
21 changes: 20 additions & 1 deletion packages/packagers/js/src/DevPackager.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
// @flow strict-local
import type {BundleGraph, PluginOptions, NamedBundle} from '@parcel/types';

import {PromiseQueue, relativeBundlePath, countLines} from '@parcel/utils';
import {
PromiseQueue,
relativeBundlePath,
countLines,
normalizeSeparators,
} from '@parcel/utils';
import SourceMap from '@parcel/source-map';
import invariant from 'assert';
import path from 'path';
Expand Down Expand Up @@ -116,6 +121,20 @@ export class DevPackager {
wrapped += JSON.stringify(deps);
wrapped += ']';

if (
this.bundle.env.isNode() &&
asset.meta.has_node_replacements === true
) {
const relPath = normalizeSeparators(
path.relative(
this.bundle.target.distDir,
path.dirname(asset.filePath),
mischnic marked this conversation as resolved.
Show resolved Hide resolved
),
);
wrapped = wrapped.replace('$parcel$dirnameReplace', relPath);
wrapped = wrapped.replace('$parcel$filenameReplace', relPath);
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason we have two different variables that get replaced with the same path? Were these meant to be different, or do we only need one of them?

}

if (this.bundle.env.sourceMap) {
if (mapBuffer) {
map.addBuffer(mapBuffer, lineOffset);
Expand Down
16 changes: 15 additions & 1 deletion packages/packagers/js/src/ScopeHoistingPackager.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,18 @@ import type {
NamedBundle,
} from '@parcel/types';

import {PromiseQueue, relativeBundlePath, countLines} from '@parcel/utils';
import {
PromiseQueue,
relativeBundlePath,
countLines,
normalizeSeparators,
} from '@parcel/utils';
import SourceMap from '@parcel/source-map';
import nullthrows from 'nullthrows';
import invariant from 'assert';
import ThrowableDiagnostic from '@parcel/diagnostic';
import globals from 'globals';
import path from 'path';

import {ESMOutputFormat} from './ESMOutputFormat';
import {CJSOutputFormat} from './CJSOutputFormat';
Expand Down Expand Up @@ -405,6 +411,14 @@ export class ScopeHoistingPackager {
this.usedHelpers.add('$parcel$global');
}

if (this.bundle.env.isNode() && asset.meta.has_node_replacements) {
const relPath = normalizeSeparators(
path.relative(this.bundle.target.distDir, path.dirname(asset.filePath)),
);
code = code.replace('$parcel$dirnameReplace', relPath);
code = code.replace('$parcel$filenameReplace', relPath);
}

let [depMap, replacements] = this.buildReplacements(asset, deps);
let [prepend, prependLines, append] = this.buildAssetPrelude(asset, deps);
if (prependLines > 0) {
Expand Down
26 changes: 26 additions & 0 deletions packages/transformers/js/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ mod fs;
mod global_replacer;
mod hoist;
mod modules;
mod node_replacer;
mod typeof_replacer;
mod utils;

Expand Down Expand Up @@ -51,6 +52,7 @@ use fs::inline_fs;
use global_replacer::GlobalReplacer;
use hoist::{hoist, CollectResult, HoistResult};
use modules::esm2cjs;
use node_replacer::NodeReplacer;
use typeof_replacer::*;
use utils::{CodeHighlight, Diagnostic, DiagnosticSeverity, SourceLocation, SourceType};

Expand All @@ -69,6 +71,7 @@ pub struct Config {
env: HashMap<swc_atoms::JsWord, swc_atoms::JsWord>,
inline_fs: bool,
insert_node_globals: bool,
node_replacer: bool,
is_browser: bool,
is_worker: bool,
is_type_script: bool,
Expand Down Expand Up @@ -102,6 +105,7 @@ pub struct TransformResult {
diagnostics: Option<Vec<Diagnostic>>,
needs_esm_helpers: bool,
used_env: HashSet<swc_atoms::JsWord>,
has_node_replacements: bool,
}

fn targets_to_versions(targets: &Option<HashMap<String, String>>) -> Option<Versions> {
Expand Down Expand Up @@ -396,6 +400,28 @@ pub fn transform(config: Config) -> Result<TransformResult, std::io::Error> {
module.fold_with(&mut passes)
};

let mut has_node_replacements = false;

let module = module.fold_with(
// Replace __dirname and __filename with placeholders in Node env
&mut Optional::new(
NodeReplacer {
source_map: &source_map,
items: &mut global_deps,
globals: HashMap::new(),
project_root: Path::new(&config.project_root),
filename: Path::new(&config.filename),
decls: &mut decls,
global_mark,
scope_hoist: config.scope_hoist,
has_node_replacements: &mut has_node_replacements,
},
config.node_replacer,
),
);

result.has_node_replacements = has_node_replacements;

let module = module.fold_with(
// Collect dependencies
&mut dependency_collector(
Expand Down
Loading