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

feature request: ava configs' factory function promise support. #1934

Closed
iamstarkov opened this issue Sep 12, 2018 · 15 comments
Closed

feature request: ava configs' factory function promise support. #1934

iamstarkov opened this issue Sep 12, 2018 · 15 comments
Labels

Comments

@iamstarkov
Copy link
Contributor

Description

The config file must have a default export, using ES modules. It can either be a plain object or a factory function which returns a plain object.

vs

We highly recommend the use of async functions. They make asynchronous code concise and readable, and they implicitly return a promise so you don't have to.

I propose to add support for a promise from a factory function, because in my case i need to look around the environment I am going to run tests for and read some files. I can use synchronous versions of fs module, but it is contrary to node's guidelines and Ava's spirit.

Test Source

export default async ({ projectDir }) => {
  /* await stuff */
  return { /* … */};
}

Error Message & Stack Trace

☯ yarn test
yarn run v1.9.4
$ ./bin test

✖ Factory method exported by ava.config.js must not return a promise

Config

N/A, see above.

Command-Line Arguments

N/A

Relevant Links

  • Sorry, project is not public

Environment

Node.js v8.11.4
darwin 17.7.0
npm 5.6.0
yarn 1.9.4
ava 1.0.0-beta.8
@novemberborn
Copy link
Member

Yup, we can do that. The restriction was added to reduce the complexity of adding this feature in the first place.

We'll have to make lib/cli work asynchronously:

ava/lib/cli.js

Line 24 in bfe1630

conf = loadConf();

We'll also need to figure out how to make this work in our ESLint plugin:

https://github.com/avajs/eslint-plugin-ava/blob/6bb239fe1cd97fd671318253debc153fab7db071/util.js#L47

@iamstarkov
Copy link
Contributor Author

sounds good

@a-deeb
Copy link

a-deeb commented Mar 20, 2019

Hi, can help out and continue where things were left off, I just made a PR #2076

@novemberborn
Copy link
Member

@a-deeb thanks, I'll have a look. If you've got questions about GitHub please comment on your PR here: #2076 there's no need to close it and open a new one.

@a-deeb
Copy link

a-deeb commented Mar 21, 2019

Thanks @novemberborn , if i have any questions ill ask here, and ill do my best to add this feature.

@a-deeb
Copy link

a-deeb commented Mar 22, 2019

Sorry for opening and closing the PR I had opened before , I did too many commits on it , the changes i made kept failing the tests, to go back to previous version or to the original is the command git checkout -f or git reset --hard or git-revert ,Also I am trying to continue where @MiguelSolano left off

@novemberborn
Copy link
Member

@a-deeb don't worry about the commit history, we can fix that when we land the PR. There's many different ways of going back to a previous version. Perhaps give GitHub Desktop a try. They've got a post on it here: https://help.github.com/en/desktop/contributing-to-projects/reverting-a-commit

This post has some good information on pushing changes: https://www.atlassian.com/git/tutorials/syncing/git-push

I can't review your code if you keep closing the PR. Just leave #2077 open and let me know when you'd like me to take a look at it.

@a-deeb
Copy link

a-deeb commented Mar 22, 2019

Thanks again @novemberborn !

@novemberborn
Copy link
Member

Our ESLint plugin now relies on AVA's config, and the ESLint plugin must be synchronous. I'm not sure where that leaves us with supporting asynchronous factories.

@iamstarkov
Copy link
Contributor Author

iamstarkov commented Jun 3, 2019

we still can support async config factories, but eslint plugin can be turned into synchronous with setTimeout or something

@novemberborn
Copy link
Member

we still can support async config factories, but eslint plugin can be turned into synchronous with setTimeout or something

You can't go from asynchronous to synchronous.

@iamstarkov
Copy link
Contributor Author

true, sorry, I had a brain fart

@iamstarkov
Copy link
Contributor Author

I'll close it then

@novemberborn
Copy link
Member

No I'd still like to support this. But we need to figure out the impact on our ESLint plugin. Maybe ESLint will support asynchronous plugins. Maybe, as long config-reliant rules are opt-in, we can make them fail when the config is asynchronous.

@novemberborn novemberborn reopened this Jun 10, 2019
@novemberborn
Copy link
Member

This will be available in AVA 4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants