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

Newly created app fails ESlint checks and fails to call test target on preact CLI #369

Closed
lambrospetrou opened this issue Sep 17, 2017 · 4 comments

Comments

@lambrospetrou
Copy link

lambrospetrou commented Sep 17, 2017

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
When I run yarn test in a newly created preact app it fails with 2 errors and 2 eslint warnings.

If the current behavior is a bug, please provide the steps to reproduce.

  1. Created a new preact app using preact create app-name --yarn
  2. Run yarn test

What is the expected behavior?
The project should conform to all defined ESLint rules.

Please mention other relevant information.

Output

yarn test
yarn test v1.0.1
$ eslint src && preact test

/path/to/app/lvb-events/lvb-events/src/components/app.js
  11:2  error  Expected line before comment  lines-around-comment

/path/to/app/lvb-events/lvb-events/src/components/header/index.js
  5:16  warning  Component should be written as a pure function  react/prefer-stateless-function

/path/to/app/lvb-events/lvb-events/src/routes/home/index.js
  4:16  warning  Component should be written as a pure function  react/prefer-stateless-function

/path/to/app/lvb-events/lvb-events/src/routes/profile/index.js
  22:2  error  updateTime should be placed before componentDidMount  react/sort-comp

✖ 4 problems (2 errors, 2 warnings)
  1 error, 0 warnings potentially fixable with the `--fix` option.

error Command failed with exit code 1.
@lambrospetrou
Copy link
Author

Also, once you fix the eslint warnings and the build succeeds you get the following failure cause preact CLI does not have a test argument.

yarn test
yarn test v1.0.1
$ eslint src && preact test
         ▄▄
     ▄▄▓▓▓▓▓▓▄▄
  ▄█▀▀█▓▓▓▓▓▓▓▀▀█▄▄
▐▓▌▐▓▓▓▒▄ ▀▄▄▓▓▓▌▐▓▌ 
▐▓▓▄▀▓▀ ▄▓▓▄▄▀▓▓ ▓▓▌ 
▐▓▓▓▌ ▒▓▌  ▐▓▓  ▓▓▓▌ preact-cli 1.4.1
▐▓▓ ▒▓▄▄▀▓▓▀ ▄▓▓ ▓▓▌
▐▓▌▐▓▓▓▀▀▄▄▀▀▓▓▓▌▐▓▌
  ▀█▄▄▒▓▓▓▓▓▓▒▄▄▒▀
      ▀▓▓▓▓▓▓▀▀
         ▀▀
For help with a specific command, enter:
  preact help [command]

Commands:
  create <name> [dest]  Create a new application.
  build [src] [dest]    Create a production build in build/
  watch [src]           Start a development live-reload server.
  serve [dir]           Start an HTTP2 static fileserver.

Options:
  --cwd       A directory to use instead of $PWD.                   [default: .]
  -h, --help  Show help                                                [boolean]

Unknown argument: test
error Command failed with exit code 1.

@lambrospetrou lambrospetrou changed the title Newly created app fails tests and ESlint checks Newly created app fails ESlint checks and fails to call test target on preact CLI Sep 17, 2017
@lambrospetrou
Copy link
Author

lambrospetrou commented Sep 17, 2017

I spend some time investigating to see how the target test: "eslint src && preact test" is added and I realised that the code adding that was removed in this commit f1891fe#diff-3a91220e73b4c8574727374e6c42763e

I don't know if it is removed on purpose since it failed or if it was just left out on error during the refactoring but preact does not have a test argument, so this should not be here anyway.

I made a PR that fixes the ESlint warnings and errors in case you want to just keep test: eslint src as a target in the generated app.

#370

And by running just the eslint, the command yarn test succeeds now:

yarn test
yarn test v1.0.1
$ eslint src
Done in 1.50s.

It would be great though in general if this starter guide was adding at least some basic tests in the example.

@reznord
Copy link
Member

reznord commented Sep 17, 2017

We are moving to custom templates in the next version of the preact-cli, so the examples in preact-cli is gonna be removed.

Regarding the test command, We are going to leave that in the user land so, the user can go ahead and use either Jest/Mocha/Chai/Enzyme and configure the app according to it.

Regarding the ESLint issue, we are planning to move to a new preset instead of using the current one.

So, in the next version all these problems will be handled gracefully. 😄

@reznord reznord closed this as completed Sep 17, 2017
@lambrospetrou
Copy link
Author

OK good to know. It's just bad first experience of the framework when creating a new app with the recommended CLI and then failing to build;p
Looking forward to trying the next version then.

Thanks

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

No branches or pull requests

2 participants