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

Fix macro issues #9485

Merged
merged 3 commits into from
Jan 14, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading