Skip to content

Commit

Permalink
Fix macro issues (#9485)
Browse files Browse the repository at this point in the history
  • Loading branch information
devongovett committed Jan 14, 2024
1 parent 9d6da2c commit 874dddf
Show file tree
Hide file tree
Showing 6 changed files with 166 additions and 6 deletions.
5 changes: 5 additions & 0 deletions crates/node-bindings/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ pub struct JsResolverOptions {
pub entries: Option<u8>,
pub extensions: Option<Vec<String>>,
pub package_exports: bool,
pub typescript: Option<bool>,
}

struct FunctionRef {
Expand Down Expand Up @@ -287,6 +288,10 @@ impl Resolver {

resolver.flags.set(Flags::EXPORTS, options.package_exports);

if matches!(options.typescript, Some(true)) {
resolver.flags |= Flags::TYPESCRIPT;
}

if let Some(module_dir_resolver) = options.module_dir_resolver {
let module_dir_resolver = FunctionRef::new(env, module_dir_resolver)?;
resolver.module_dir_resolver = Some(Arc::new(move |module: &str, from: &Path| {
Expand Down
67 changes: 66 additions & 1 deletion packages/core/integration-tests/test/macros.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ describe('macros', function () {
it('should support named imports', async function () {
await fsFixture(overlayFS, dir)`
index.js:
import { hash } from "./macro.js" with { type: "macro" };
import { hash } from "./macro" with { type: "macro" };
output = hash('hi');
macro.js:
Expand Down Expand Up @@ -340,6 +340,50 @@ describe('macros', function () {
}
});

it('should throw a diagnostic when a macro cannot be resolved', async function () {
await fsFixture(overlayFS, dir)`
index.js:
import { test } from "./macro.js" with { type: "macro" };
output = test(1, 2);
`;

try {
await bundle(path.join(dir, '/index.js'), {
inputFS: overlayFS,
mode: 'production',
});
} catch (err) {
assert.deepEqual(err.diagnostics, [
{
message: `Error evaluating macro: Could not resolve module "./macro.js" from "${path.join(
dir,
'index.js',
)}"`,
origin: '@parcel/transformer-js',
codeFrames: [
{
filePath: path.join(dir, 'index.js'),
codeHighlights: [
{
message: undefined,
start: {
line: 2,
column: 10,
},
end: {
line: 2,
column: 19,
},
},
],
},
],
hints: null,
},
]);
}
});

it('should support returning functions', async function () {
await fsFixture(overlayFS, dir)`
index.js:
Expand Down Expand Up @@ -382,6 +426,27 @@ describe('macros', function () {
assert(res.includes('output=3'));
});

it('should support macros written in typescript without extension', async function () {
await fsFixture(overlayFS, dir)`
index.js:
import { test } from "./macro" with { type: "macro" };
output = test(1, 2);
macro.ts:
export function test(a: number, b: number) {
return a + b;
}
`;

let b = await bundle(path.join(dir, '/index.js'), {
inputFS: overlayFS,
mode: 'production',
});

let res = await overlayFS.readFile(b.getBundles()[0].filePath, 'utf8');
assert(res.includes('output=3'));
});

it('should allow emitting additional assets', async function () {
await fsFixture(overlayFS, dir)`
index.js:
Expand Down
1 change: 1 addition & 0 deletions packages/core/package-manager/src/NodePackageManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ export class NodePackageManager implements PackageManager {
}
: undefined,
extensions: this.currentExtensions,
typescript: true,
});
}

Expand Down
80 changes: 77 additions & 3 deletions packages/core/package-manager/test/NodePackageManager.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,32 @@ import {MockPackageInstaller, NodePackageManager} from '../src';

const FIXTURES_DIR = path.join(__dirname, 'fixtures');

function normalize(res) {
return {
...res,
invalidateOnFileCreate:
res?.invalidateOnFileCreate?.sort((a, b) => {
let ax =
a.filePath ??
a.glob ??
(a.aboveFilePath != null && a.fileName != null
? a.aboveFilePath + a.fileName
: '');
let bx =
b.filePath ??
b.glob ??
(b.aboveFilePath != null && b.fileName != null
? b.aboveFilePath + b.fileName
: '');
return ax < bx ? -1 : 1;
}) ?? [],
};
}

function check(resolved, expected) {
assert.deepEqual(normalize(resolved), normalize(expected));
}

describe('NodePackageManager', function () {
let fs;
let packageManager;
Expand All @@ -36,7 +62,7 @@ describe('NodePackageManager', function () {
});

it('resolves packages that exist', async () => {
assert.deepEqual(
check(
await packageManager.resolve(
'foo',
path.join(FIXTURES_DIR, 'has-foo/index.js'),
Expand All @@ -51,10 +77,26 @@ describe('NodePackageManager', function () {
path.join(FIXTURES_DIR, 'has-foo/node_modules/foo/package.json'),
]),
invalidateOnFileCreate: [
{
filePath: path.join(
FIXTURES_DIR,
'has-foo/node_modules/foo/index.ts',
),
},
{
filePath: path.join(
FIXTURES_DIR,
'has-foo/node_modules/foo/index.tsx',
),
},
{
fileName: 'node_modules/foo',
aboveFilePath: path.join(FIXTURES_DIR, 'has-foo'),
},
{
fileName: 'tsconfig.json',
aboveFilePath: path.join(FIXTURES_DIR, 'has-foo'),
},
],
},
);
Expand All @@ -73,7 +115,7 @@ describe('NodePackageManager', function () {
it("autoinstalls packages that don't exist", async () => {
packageInstaller.register('a', fs, path.join(FIXTURES_DIR, 'packages/a'));

assert.deepEqual(
check(
await packageManager.resolve(
'a',
path.join(FIXTURES_DIR, 'has-foo/index.js'),
Expand All @@ -89,10 +131,26 @@ describe('NodePackageManager', function () {
path.join(FIXTURES_DIR, 'has-foo/node_modules/a/package.json'),
]),
invalidateOnFileCreate: [
{
filePath: path.join(
FIXTURES_DIR,
'has-foo/node_modules/a/index.ts',
),
},
{
filePath: path.join(
FIXTURES_DIR,
'has-foo/node_modules/a/index.tsx',
),
},
{
fileName: 'node_modules/a',
aboveFilePath: path.join(FIXTURES_DIR, 'has-foo'),
},
{
fileName: 'tsconfig.json',
aboveFilePath: path.join(FIXTURES_DIR, 'has-foo'),
},
],
},
);
Expand Down Expand Up @@ -219,7 +277,7 @@ describe('NodePackageManager', function () {
);

let spy = sinon.spy(packageInstaller, 'install');
assert.deepEqual(
check(
await packageManager.resolve(
'foo',
path.join(FIXTURES_DIR, 'has-foo/subpackage/index.js'),
Expand All @@ -245,10 +303,26 @@ describe('NodePackageManager', function () {
),
]),
invalidateOnFileCreate: [
{
filePath: path.join(
FIXTURES_DIR,
'has-foo/subpackage/node_modules/foo/index.ts',
),
},
{
filePath: path.join(
FIXTURES_DIR,
'has-foo/subpackage/node_modules/foo/index.tsx',
),
},
{
fileName: 'node_modules/foo',
aboveFilePath: path.join(FIXTURES_DIR, 'has-foo/subpackage'),
},
{
fileName: 'tsconfig.json',
aboveFilePath: path.join(FIXTURES_DIR, 'has-foo/subpackage'),
},
],
},
);
Expand Down
2 changes: 1 addition & 1 deletion packages/transformers/js/src/JSTransformer.js
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ export default (new Transformer({
}
} catch (err) {
// Remove parcel core from stack and build string so Rust can process errors more easily.
let stack = err.stack.split('\n').slice(1);
let stack = (err.stack || '').split('\n').slice(1);
let message = err.message;
for (let line of stack) {
if (line.includes(__filename)) {
Expand Down
17 changes: 16 additions & 1 deletion packages/utils/node-resolver-rs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ bitflags! {
/// Optional extensions in specifiers, using the `extensions` setting.
const OPTIONAL_EXTENSIONS = 1 << 7;
/// Whether extensions are replaced in specifiers, e.g. `./foo.js` -> `./foo.ts`.
/// This also allows omitting the `.ts` and `.tsx` extensions when outside node_modules.
const TYPESCRIPT_EXTENSIONS = 1 << 8;
/// Whether to allow omitting the extension when resolving the same file type.
const PARENT_EXTENSION = 1 << 9;
Expand Down Expand Up @@ -166,7 +167,7 @@ impl<'a, Fs: FileSystem> Resolver<'a, Fs> {
pub fn parcel(project_root: Cow<'a, Path>, cache: CacheCow<'a, Fs>) -> Self {
Self {
project_root,
extensions: Extensions::Borrowed(&["ts", "tsx", "mjs", "js", "jsx", "cjs", "json"]),
extensions: Extensions::Borrowed(&["mjs", "js", "jsx", "cjs", "json"]),
index_file: "index",
entries: Fields::MAIN | Fields::SOURCE | Fields::BROWSER | Fields::MODULE,
flags: Flags::all(),
Expand Down Expand Up @@ -876,6 +877,20 @@ impl<'a, Fs: FileSystem> ResolveRequest<'a, Fs> {
}
}

// Try adding typescript extensions if outside node_modules.
if self
.resolver
.flags
.contains(Flags::TYPESCRIPT_EXTENSIONS | Flags::OPTIONAL_EXTENSIONS)
&& !self.flags.contains(RequestFlags::IN_NODE_MODULES)
{
if let Some(res) =
self.try_extensions(path, package, &Extensions::Borrowed(&["ts", "tsx"]), true)?
{
return Ok(Some(res));
}
}

// Try appending the configured extensions.
if let Some(res) = self.try_extensions(path, package, &self.resolver.extensions, true)? {
return Ok(Some(res));
Expand Down

0 comments on commit 874dddf

Please sign in to comment.