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

Don't reach into build folder of jest-resolve #1

Closed
SimenB opened this issue Oct 31, 2018 · 10 comments · Fixed by jestjs/jest#7714
Closed

Don't reach into build folder of jest-resolve #1

SimenB opened this issue Oct 31, 2018 · 10 comments · Fixed by jestjs/jest#7714

Comments

@SimenB
Copy link

SimenB commented Oct 31, 2018

Jest is currently renaming files: jestjs/jest#4969

The default resolver should probably be exported from main

@arcanis
Copy link
Owner

arcanis commented Nov 1, 2018

I think a better option might be for jest-resolve to default to either:

  • pass the default resolver as an option to the custom resolver, and/or
  • use the default resolver when the custom resolver's module.exports is not a function

In both of these cases, jest-pnp-resolver could then disable itself without having to keep a dependency to jest-resolve. How does that sound?

@SimenB
Copy link
Author

SimenB commented Nov 1, 2018

The first one sounds good to me, feels more explicit and allows people to implement a resolver that can fall back to default behavior if they cannot find their module for whatever reason.

@SimenB
Copy link
Author

SimenB commented Feb 5, 2019

Out in 24.1.0

@arcanis
Copy link
Owner

arcanis commented Feb 5, 2019

@SimenB fyi I was thinking I could keep the ugly thing until most of the ecosystem switches on Jest 24 (so probably until Jest 25, I'd say). Does that sound ok?

@SimenB
Copy link
Author

SimenB commented Feb 5, 2019

Sure! However, you can check if you get it passed in now, and only do your require if not?

@arcanis
Copy link
Owner

arcanis commented Feb 5, 2019

Hmm that might work yeah - it adds a slight indirection but it shouldn't be a big deal in most cases 👍

@terwort
Copy link

terwort commented Feb 5, 2019

Hey guys, I'm trying to update to jest 24, and I'm getting the error about how it can't find the old path (default_resolver). I saw in index.js you implemented the ugly thing, but in your latest release, 1.0.2, this code isn't there. Any chance the fix will get released soon?

@terwort
Copy link

terwort commented Feb 5, 2019

Ahh.. just noticed you referenced a PR that will close this problem. Any idea on release time?

@arcanis arcanis closed this as completed in #6 Feb 5, 2019
@arcanis
Copy link
Owner

arcanis commented Feb 5, 2019

@terwort I've just merged and released the change as 1.1.0 - can you confirm the fix? Seems to work for me.

@terwort
Copy link

terwort commented Feb 5, 2019

@arcanis I can confirm it is fixed! Thank you very much for your fast delivery 😄

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 a pull request may close this issue.

3 participants