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 #14 #15

Merged
merged 1 commit into from
May 9, 2023
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
19 changes: 19 additions & 0 deletions packages/forgetti/src/core/checks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,22 @@ export function isNestedExpression(node: t.Node): node is NestedExpression {
return false;
}
}

/** Check whether a Node is CallExpression and a React Hook call */
export function isCallExpressionAndHook(ctx: StateContext, node: t.Node): node is t.CallExpression {
if (node.type !== 'CallExpression') {
return false;
}

// A simple check to see if "node.callee" is an Identifier
// TypeScript is able to infer the type of "node.callee" to Identifier or V8IntrinsicIdentifier
if (!('name' in node.callee)) {
return false;
}

// Check if callee is a react hook
if (!ctx.filters.hook) {
return false;
}
return ctx.filters.hook.test(node.callee.name);
}
6 changes: 5 additions & 1 deletion packages/forgetti/src/core/optimizer.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-disable @typescript-eslint/no-unnecessary-condition */
import type * as babel from '@babel/core';
import * as t from '@babel/types';
import { isNestedExpression, isPathValid } from './checks';
import { isCallExpressionAndHook, isNestedExpression, isPathValid } from './checks';
import getForeignBindings, { isForeignBinding } from './get-foreign-bindings';
import getImportIdentifier from './get-import-identifier';
import { RUNTIME_EQUALS } from './imports';
Expand Down Expand Up @@ -561,6 +561,10 @@ export default class Optimizer {
for (let i = 0, len = argumentsPath.length; i < len; i++) {
argument = argumentsPath[i];
if (isPathValid(argument, t.isExpression)) {
if (isCallExpressionAndHook(this.ctx, argument.node)) {
continue;
}

const optimized = this.createDependency(argument);
if (optimized) {
mergeDependencies(dependencies, optimized.deps);
Expand Down
9 changes: 9 additions & 0 deletions packages/forgetti/test/__snapshots__/hooks.test.ts.snap
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
// Vitest Snapshot v1

exports[`hooks > should correct transform nested hooks call (issue #14) 1`] = `
"import { useDeferredValue } from 'react';
import { useAtomValue } from 'jotai';
import { stateAtom } from 'whatever';
function Example(props) {
return useDeferredValue(useAtomValue(stateAtom));
}"
`;

exports[`hooks > should optimize useCallback 1`] = `
"import { useMemo as _useMemo } from \\"react\\";
import { $$cache as _$$cache } from \\"forgetti/runtime\\";
Expand Down
11 changes: 11 additions & 0 deletions packages/forgetti/test/hooks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,17 @@ import { useEffect } from 'react';
function Example(props) {
useEffect(() => props.value());
}
`;
expect(await compile(code)).toMatchSnapshot();
});
it('should correct transform nested hooks call (issue #14)', async () => {
const code = `
import { useDeferredValue } from 'react';
import { useAtomValue } from 'jotai';
import { stateAtom } from 'whatever';
function Example(props) {
return useDeferredValue(useAtomValue(stateAtom));
}
`;
expect(await compile(code)).toMatchSnapshot();
});
Expand Down