Hoist ava to the root #276
Conversation
| @@ -3,11 +3,11 @@ | |||
| "scripts": { | |||
| "lint": "eslint \"**/*.js\"", | |||
| "format": "prettier \"**/*.{js,ts,tsx}\" --write --loglevel warn", | |||
| "test": "prettier \"**/*.{js,ts,tsx}\" --list-different && yarn lint && lerna run test" | |||
| "test": "prettier \"**/*.{js,ts,tsx}\" --list-different && yarn lint && ava" | |||
There was a problem hiding this comment.
I know, it's not exactly the scope of this PR, but:
How about moving yarn lint, for instance, into a posttest script? Might make the scripts easier readable. Thoughts?
There was a problem hiding this comment.
Linting is a lot faster than running tests but it often catches errors. I think it's good to fail early.
There was a problem hiding this comment.
How about moving it to pretest then? 😉
(Btw, this is just a minor nitpick. I just thought it might be a tiny improvement. Feel free to disagree.)
There was a problem hiding this comment.
{
"test": "prettier \"**/*.{js,ts,tsx}\" --list-different && yarn lint && ava"
}
vs
{
"pretest": "prettier \"**/*.{js,ts,tsx}\" --list-different && yarn lint",
"test": "ava"
}
It barely improves anything 😄 If you want to make it shorter I can replace --list-different with -l.
| @@ -9,33 +9,16 @@ Generated by [AVA](https://ava.li). | |||
| > Snapshot 1 | |||
|
|
|||
| { | |||
| devServer: { | |||
There was a problem hiding this comment.
It changed when I only updated ava from 0.22.0 to 0.25.0 before doing any other changes. I couldn't find any release notes related to it.
I'll have look on how to fix it.
There was a problem hiding this comment.
Fixed by setting NODE_ENV to development. This was broken because since 0.23.0 ava sets NODE_ENV to test.
| @@ -8,7 +8,23 @@ const configFile = require.resolve('../webpack.config.babel') | |||
|
|
|||
| test('it exports the development config', t => { | |||
| const originalConfig = requireConfig() | |||
| const module = Object.assign({}, originalConfig.module, { | |||
There was a problem hiding this comment.
Didn't really expect that change here. It's surely not wrong, but it there a background story why you added this just now? (Just curious :) )
There was a problem hiding this comment.
Because I made a change in packages/sample-app/webpack.config.babel.js:
- typescript(),
+ typescript({ configFileName: path.resolve(__dirname, './tsconfig.json') }),
| @@ -5,10 +5,6 @@ | |||
| "main": "lib/index", | |||
| "license": "MIT", | |||
| "author": "Andy Wermke <andy@dev.next-step-software.com>", | |||
| "scripts": { | |||
| "test": "ava", | |||
| "prepublishOnly": "yarn test" | |||
There was a problem hiding this comment.
Do we still get tests prior to release?
There was a problem hiding this comment.
Nope, manual only for now. I would like to tickle it in another PR.
- Publishing single packages leads to version inconsistencies and bugs. For instance the
uglifyblock was publish separately as 1.0.0 and later as 1.1.0, when other blocks used to be v1.0.0-beta. Then with the 1.0 release theuglifywas republished and its version 1.0.0 was tagged as latest. What that means is if you runyarn add @webpack-blocks/uglifynow, you will get 1.0.0 instead of 1.1.0. (I am sorry I haven't reported it before 😅) - Some blocks have no unit but instead have related e2e tests. I believe they are more important to have and we can focus on writing more of them.
There was a problem hiding this comment.
Oh okay. Then we should just create a small doc to refer to when publishing. Like that we only publish "whole monorepo", not standalone.
| @@ -1,4 +1,4 @@ | |||
| # Snapshot report for `__tests__/webpack-config.test.js` | |||
| # Snapshot report for `packages\sample-app\__tests__\webpack-config.test.js` | |||
There was a problem hiding this comment.
Are you running the tests from a windows machine? Wondering because (back)slashes keeps changing back & forth
There was a problem hiding this comment.
Nice catch! I've just regenerated snapshots from my Mac.
And also update it to the latest.