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

Add @interactors/cli #264

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open

Add @interactors/cli #264

wants to merge 25 commits into from

Conversation

wKich
Copy link
Member

@wKich wKich commented May 22, 2024

Motivation

First step towards to bigtest v2. Changes to generate environmental agent.ts with desired interactors

Approach

Changed initialisation for matchers through using createMatcher and added abstract classes for matchers and interactors to correctly identify defined them
Added @interactors/cli with ability to generate agent.ts

TODOs and Open Questions

  • Missing ok and err function from manually written agent.ts

@wKich wKich requested review from cowboyd, taras and jbolda May 22, 2024 13:24
Copy link

changeset-bot bot commented May 22, 2024

🦋 Changeset detected

Latest commit: c67dbfa

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

This PR includes changesets to release 4 packages
Name Type
@interactors/core Minor
@interactors/keyboard Patch
@interactors/html Patch
@interactors/material-ui Patch

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

Copy link

netlify bot commented May 22, 2024

Deploy Preview for interactors canceled.

Name Link
🔨 Latest commit c67dbfa
🔍 Latest deploy log https://app.netlify.com/sites/interactors/deploys/6697a280ed534b000824dfbe

Copy link
Member

@taras taras left a comment

Choose a reason for hiding this comment

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

🔥 I'll leave it to @cowboyd to do the review but this is exciting

@cowboyd
Copy link
Member

cowboyd commented May 22, 2024

@wKich This is looking awesome!!!

I'm thinking that we should perhaps make only a single global property on the globalThis object. This means that among other things we're concentrated on a single place, and also, we can make a nice TypeScript interface for it. What about:

interface InteractorAgent {
  run(interaction: Interaction): Result<void>;
  interactors: Record<string, InteractorConstructor>;
  matchers: Record<string, MatcherConstructor>;
};

The other thing I'm thinking is that the rematch() function is a little too clever. We should make every value that is processes be boxed into a type that is reified by the agent. This will mean that rematch can be made into a generic construct() that construct anything.

The next step is that we need to generate the constructors that will be used on the client side to send the data over to the agent.

@cowboyd
Copy link
Member

cowboyd commented May 30, 2024

I've opened a ticket #265 to have a more complete picture of what the CLI should accomplish. Let's start with the build command and we can go from there.

Copy link
Member

@cowboyd cowboyd left a comment

Choose a reason for hiding this comment

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

I tried to use it today and was almost successful, but not quite.

  • For simplicity's sake, Let's get rid of interactors.config.js for now and say that everything has to be specified on the command line via arguments and options.
  • The directory loading using dynamic import() will load relative to the module that is calling it, so we can't use relative paths. Instead, we want to import relative to the directory from which the CLI was invoked, so we should import absolute paths that are resolved relative to process.cwd()
  • The opposite is true of the agent.template.ts. It is being read relative to process.cwd(), but we need to copy the agent template to the dist directory as part of the build and read it relative to the interactors cli sources.

Comment on lines 3 to 4
import { matcherDescription } from './matcher';
import { MaybeMatcher } from '../dist';
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
import { matcherDescription } from './matcher';
import { MaybeMatcher } from '../dist';
import { MaybeMatcher, matcherDescription } from './matcher';

Copy link
Member

Choose a reason for hiding this comment

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

This was causing the build to fail. Looks like maybe VSCode was importing this automatically.

wKich and others added 19 commits July 17, 2024 15:46
Signed-off-by: Dmitriy Lazarev <w@kich.dev>
Signed-off-by: Dmitriy Lazarev <w@kich.dev>
Signed-off-by: Dmitriy Lazarev <w@kich.dev>
Signed-off-by: Dmitriy Lazarev <w@kich.dev>
Signed-off-by: Dmitriy Lazarev <w@kich.dev>
Signed-off-by: Dmitriy Lazarev <w@kich.dev>
As part of adding the `dev` command, this makes each command have
its own file.
Signed-off-by: Dmitriy Lazarev <w@kich.dev>
Signed-off-by: Dmitriy Lazarev <w@kich.dev>
Signed-off-by: Dmitriy Lazarev <w@kich.dev>
Signed-off-by: Dmitriy Lazarev <w@kich.dev>
Signed-off-by: Dmitriy Lazarev <w@kich.dev>
Signed-off-by: Dmitriy Lazarev <w@kich.dev>
Signed-off-by: Dmitriy Lazarev <w@kich.dev>
Signed-off-by: Dmitriy Lazarev <w@kich.dev>
Signed-off-by: Dmitriy Lazarev <w@kich.dev>
Signed-off-by: Dmitriy Lazarev <w@kich.dev>
@wKich wKich mentioned this pull request Aug 8, 2024
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants