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

Passed filePath to dependencyExtractor #7362

Merged

Conversation

rubennorte
Copy link
Contributor

Summary

The dependency extraction might depend on the filename (some Babel plugins dynamically add dependencies using the filename). This passes an additional parameter to the extract method (which is breaking but this hasn't been released in any stable yet).

Test plan

Updated unit and e2e tests.

@rubennorte rubennorte force-pushed the pass-filename-to-dependency-extractor branch from 3dfc38b to 5e950de Compare November 13, 2018 18:31
@rubennorte rubennorte force-pushed the pass-filename-to-dependency-extractor branch from 5e950de to a5fc6e3 Compare November 13, 2018 18:48
@rubennorte rubennorte force-pushed the pass-filename-to-dependency-extractor branch from a5fc6e3 to a4b1e36 Compare November 13, 2018 18:54
const crypto = require('crypto');

module.exports = {
extract(code, filePath, defaultExtract) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we pass an object instead of multiple args? Or code as first, then an object?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would've preferred that (I generally prefer objects than positional arguments as they're easier to extend) but I favored consistency with the rest of the Jest codebase.

Now that we're considering how the configuration should be from now on, we can reach an agreement about this.

This isn't urgent so we can discuss this in a wider scope before merging.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My vote is an object :D

@SimenB SimenB added this to the Jest 24 milestone Jan 9, 2019
@SimenB
Copy link
Member

SimenB commented Jan 9, 2019

@rubennorte land this? 🙂

@rubennorte rubennorte force-pushed the pass-filename-to-dependency-extractor branch from a4b1e36 to efaa14f Compare January 14, 2019 11:11
@rubennorte rubennorte merged commit 258c170 into jestjs:master Jan 14, 2019
@rubennorte rubennorte deleted the pass-filename-to-dependency-extractor branch January 14, 2019 11:28
captain-yossarian pushed a commit to captain-yossarian/jest that referenced this pull request Jul 18, 2019
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants