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

jest can't find babel when i'm using gitDir #158

Closed
bmsuseluda opened this issue May 2, 2017 · 16 comments
Closed

jest can't find babel when i'm using gitDir #158

bmsuseluda opened this issue May 2, 2017 · 16 comments

Comments

@bmsuseluda
Copy link

bmsuseluda commented May 2, 2017

Hello okonet,
thank you for your great work.
I'm trying to use jest with lint staged, but when i'm using the "gitDir" option jest can't find babel-jest and so every test fails.

My config:

{
    "gitDir": "../",
    "linters": {
        "*.js": [
            "prettier --single-quote --trailing-comma all --write",
            "git add",
            "eslint --ignore-path ./gui/.gitignore",
            "jest --config ./gui/jest.json --bail --findRelatedTests"
        ],
        "*.css": [
            "stylelint"
        ]
    }
}

My folder structure:
gitFolder -> gui (frontend-app)

^^^^^^ SyntaxError: Unexpected token import at transformAndBuildScript (gui/node_modules/jest-runtime/build/transform.js:320:12)

I think the problem is that jest can't find babel-jest, because of the "gitDir"-option and so can't transform the import statement.

When i'm using jest without lint-staged it's working like a charm.

lint-staged: 3.4.0
jest: 19.0.2
babel-jest: 19.0.0
husky: 0.13.3

I hope you've got an idea where the problem comes from.
Thank you

@okonet
Copy link
Collaborator

okonet commented May 2, 2017

I'm not sure how this can cause this issue. Probably a way how Jest resolves babel-jest?

@bmsuseluda
Copy link
Author

bmsuseluda commented May 2, 2017

When i'm using the following option in my jest.json jest finds babel-jest:

"transform": {
    "^.+\\.js?$": "./gui/node_modules/babel-jest"
  },

the default for this in jest is:

"transform": {
    "^.+\\.js?$": "babel-jest"
  },

But now every other option in my jest.json points at the gitFolder and not the subfolder gui.
I was playing with the jest option "rootDir" to solve this, but than, jest can't resolve the files from lint-staged.

Correct me if i'm wrong: I think a general problem with "gitDir" is, that the linters will be executed in the gitDir and not in the frontend-app (npm project).
You can see this problem in my eslint config aswell:
"eslint --ignore-path ./gui/.gitignore" instead of "eslint --ignore-path .gitignore"

@okonet
Copy link
Collaborator

okonet commented May 3, 2017

AFAIK executables should be located relative to package.json and node_modules directory in order to work but all paths provided to them are absolute to make gitDir option work but since I'm not using this option myself I might be wrong.

@bmsuseluda
Copy link
Author

I made another test to isolate the problem. I put the jest config in my package.json, so that i don't need the config-parameter and removed every linting tool, but jest.
My config is than:

{
    "gitDir": "../",
    "linters": {
        "*.js": [
            "jest --findRelatedTests"
        ]
    }
}

The result is that jest can't find my config at all:

TypeError: Cannot read property 'config' of null
    at loadFromPackage.then ([path removed for issue report]/app/gui/node_modules/jest-config/build/index.js:50:38)

@okonet
Copy link
Collaborator

okonet commented May 3, 2017

Can you add "verbose": true after gitDir to get more output? Can you also check out how is jest got called? Another idea to try: create a script `"run-jest": "jest --findRelatedTests" and use it in the config instead of calling jest directly.

@okonet
Copy link
Collaborator

okonet commented May 3, 2017

Can you create a GH repo with this setup so I can play around with it?

@bmsuseluda
Copy link
Author

The idea to call a npm-script instead of jest directly was a good one. It's working =D
Now all jest internal parameters got the right path.
Thank you very much.

@okonet
Copy link
Collaborator

okonet commented May 3, 2017

Great. It would be still good to get to the root cause so if you can invest some time into it it would be great!

@bmsuseluda
Copy link
Author

bmsuseluda commented May 3, 2017

https://github.com/bmsuseluda/lintStagedJestTest/tree/master/gui
Here you can find a simple project with just 1 snapshot test. If you change the test you can see the problem with npm run precommit

@okonet
Copy link
Collaborator

okonet commented May 8, 2017

I believe this line makes it behave like this: https://github.com/okonet/lint-staged/blob/master/src/runScript.js#L14

@DeFuex
Copy link
Contributor

DeFuex commented May 8, 2017

hi @bmsuseluda , i don't know if the issue still persists, but i'm not getting the problem by changing the included test and running npm run precommit...is there any step i might have forgotten to get to the problem?

@bmsuseluda
Copy link
Author

bmsuseluda commented May 8, 2017

Hi @DeFuex,
in the test i recommend to change the button id to something different for example "fooBar".
After changing the test you have to add the change to git with git add . before you are running npm run precommit. This is necessary for lint-staged to see the edited files.

@bmsuseluda
Copy link
Author

bmsuseluda commented May 8, 2017

Hi @okonet thanks for the commit. In my opinion it's better to reverse the algorithm from
res.bin !== 'npm' && options && options.gitDir ? { cwd: options.gitDir } : {}
to
res.bin === 'git' && options && options.gitDir ? { cwd: options.gitDir } : {}
so every execution i can do in the package.json would work in lintStaged.
Just the git executions like git add need the gitDir option.

@DeFuex
Copy link
Contributor

DeFuex commented May 8, 2017

@bmsuseluda yeah, im a little bit tired today, seeing the issue now. had some other difficulties 😄

@okonet
Copy link
Collaborator

okonet commented May 8, 2017

@bmsuseluda good idea! Do you or @DeFuex mind creating a PR for it? I'd add a long-ish comment explaining why it's needed. This should go away when #75 is done (if it's even possible, I still don't know)

@bmsuseluda
Copy link
Author

@okonet thanks, i will do it tomorrow.

billyvg pushed a commit to billyvg/lint-staged that referenced this issue May 16, 2017
Inverses logic of checking for "not equal to `npm`", instead check
that binary to run is equal to `git` (and accounting for absolute
paths). This means that commands besides `npm` will also run in CWD
instead of `gitDir`.

Closes lint-staged#158
okonet pushed a commit that referenced this issue May 17, 2017
* change npm to git since gitDir is only used for git executables

* fix(gitDir): `gitDir` should only be applied when using `git` binary

Inverses logic of checking for "not equal to `npm`", instead check
that binary to run is equal to `git` (and accounting for absolute
paths). This means that commands besides `npm` will also run in CWD
instead of `gitDir`.

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

No branches or pull requests

3 participants