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

Export function of child_process #110

Merged
merged 2 commits into from
Apr 28, 2020
Merged

Conversation

minkimcello
Copy link
Collaborator

@minkimcello minkimcello commented Apr 27, 2020

Motivation

This comment.

Approach

I think I may be underestimating/misunderstanding the issue but can't we just do:

// > packages/node/src/index.ts

export * as ChildProcess from './child_process';

This seems to allow us to:

import { ChildProcess } from `@effection/node`;

export function * echo() {
  yield ChildProcess.fork("echo", ["Hello World"]);
}

Not knowing how effection works I'm not sure how I can test to make sure it doesn't conflict with @effection's fork but hovering over the imported functions seems to display the intended result:

Screen Shot 2020-04-27 at 3 37 43 PM

@cowboyd
Copy link
Member

cowboyd commented Apr 27, 2020

What about the other forms?

import { spawn } from `@effection/node/child_process`
import * as ChildProcess from `@effection/node/child_process`

@minkimcello
Copy link
Collaborator Author

@cowboyd I believe both of those forms work if you specify the dist directory in that path.

Screen Shot 2020-04-27 at 6 51 07 PM

// > node/src/index.ts
export { spawn, fork } from './child_process';

☝️ Was this conflicting with fork from @effection?
Because when it's exported and I import it as:

import { fork, spawn } from '@effection/node';

It looks to be importing the correct function.

@cowboyd
Copy link
Member

cowboyd commented Apr 27, 2020

I believe both of those forms work if you specify the dist directory in that path.

Is there a way to do it without having to put dist/ in the middle?

@minkimcello
Copy link
Collaborator Author

@cowboyd Went down a bit of a rabbit hole and found these issues: #10866, #26722, and #33079. I think these people are mentioning what it is that we want to do with @effection/node/child_process.

Could we use something like module-alias?

@cowboyd
Copy link
Member

cowboyd commented Apr 28, 2020

Based on the last ticket, It seems like for node > 13 we can use package exports coupled with tsconfig paths. But maybe we can add that later?

We got burned with module-alias before so not keen to go down that road,

@cowboyd
Copy link
Member

cowboyd commented Apr 28, 2020

@minkimcello Thanks for going down these rabbit holes, otherwise known as "research." This is exactly the information that we need.

@minkimcello minkimcello merged commit 45cc4f6 into child_process Apr 28, 2020
@minkimcello minkimcello deleted the mk/typescript-child branch April 28, 2020 20:01
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.

2 participants