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.

Sign up for GitHub

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

Jump to bottom

check action objects for implementation use #395

Closed
wants to merge 2 commits into from
Closed

check action objects for implementation use #395

wants to merge 2 commits into from

Conversation

with-heart
Copy link
Contributor

@with-heart with-heart commented Nov 8, 2023 •
edited
Loading

Resolves #355
Resolves #373

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Nov 8, 2023 •
edited
Loading

🦋 Changeset detected

Latest commit: 1816004

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 0 packages

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Andarist
Andarist reviewed Nov 8, 2023
View reviewed changes
apps/extension/server/src/diagnostics/getUnusedActionImplementations.ts
@@ -9,7 +9,7 @@ export const getUnusedActionImplementations: DiagnosticGetter = (
machineResult,
) => {
const allActions = getSetOfNames(
machineResult.getAllActions(['named']) || [],
machineResult.getAllActions(['named', 'object']) || [],
Copy link
Member

Choose a reason for hiding this comment

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

The 'object' declaration type doesn't exist:

xstate-tools/packages/machine-extractor/src/MachineExtractResult.ts

Lines 603 to 608 in d9cc11d

declarationTypes: DeclarationType[] = [
'identifier',
'inline',
'unknown',
'named',
],

was this maybe intended as a draft PR? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does exist, that default array is just confusing

xstate-tools/packages/machine-extractor/src/types.ts

Lines 87 to 92 in d9cc11d

export type DeclarationType =
| 'named'
| 'inline'
| 'identifier'
| 'object'
| 'unknown';

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Used over here

xstate-tools/packages/machine-extractor/src/actions.ts

Lines 121 to 142 in d9cc11d

export const ActionAsObjectExpression = createParser({
babelMatcher: t.isObjectExpression,
parseNode: (node, context): ActionNode => {
const id = context.getNodeHash(node);
for (const prop of node.properties) {
if (t.isObjectProperty(prop)) {
if (
getObjectPropertyKey(prop) === 'type' &&
t.isStringLiteral(prop.value) &&
!SUPPORTED_BUILTIN_ACTIONS.includes(prop.value.value)
) {
return {
action: id,
node,
name: prop.value.value,
kind: 'named',
declarationType: 'object',
inlineDeclarationId: id,
};
}
}
}

@with-heart with-heart closed this Jul 26, 2024
@with-heart with-heart deleted the check-action-objects-for-implementation-use branch July 26, 2024 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Andarist Andarist Andarist left review comments

@davidkpiano davidkpiano davidkpiano approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@with-heart @davidkpiano @Andarist