-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Add support for the "module" package.json field #2704
Conversation
Generated by 🚫 dangerJS |
Codecov Report@@ Coverage Diff @@
## master #2704 +/- ##
==========================================
- Coverage 67.51% 67.43% -0.08%
==========================================
Files 142 142
Lines 5094 5101 +7
==========================================
+ Hits 3439 3440 +1
- Misses 1655 1661 +6
Continue to review full report at Codecov.
|
I added a test 🤷♂️ |
1aa019a
to
0b85995
Compare
@cpojer could you review? :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you also add a brief explanation in docs about this and why is it helpful?
I'd also ask for a little patience, because we're focusing on rolling out new release.
packages/jest-resolve/src/index.js
Outdated
path, | ||
{ | ||
basedir: options.basedir, | ||
extensions: options.extensions, | ||
moduleDirectory: options.moduleDirectory, | ||
packageFilter: options.module ? packageJson => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit, Jest uses this style:
condition
? truth
: false
c3424c0
to
d5356c2
Compare
@thymikee All set. I'd love to get this in the next release if possible... it helps greatly when working on lerna codebases so you don't have to prebuild every package before running tests on packages with deps on other packages. |
9176528
to
8f9514a
Compare
Rebased |
Hey @yaycmyk. Thanks for your PR. Can you go into more detail where this is used and how this is a standard? We are planning on releasing Jest 19 next week but I'm afraid if you want to get this in, I'd ask you to make a breaking change to Jest to make sure we aren't adding more options. I would be happy to support this feature and the browser resolution feature if they are merged into one config, like this:
This would prefer the "module" field over the "browser" field or the default. This will also enable things like react-native support, this way:
What do you think? |
|
@cpojer one wrinkle here: browser is actually a different data structure. "main" and "module" are a single file path, whereas browser is an object of paths/module names pointing to other paths. For example: {
"browser": {
"build/foo.js": "build/foo.umd.js"
},
"main": "build/foo.js",
"module": "foo.js"
} I suppose I could write a resolver that handles both cases, but it would require more extensive changes. |
I tested the regex thoroughly, so I think it's just a matter of usage. Regex.test() has some weird semantics if its run over the same string twice... so switched to match() instead.
This is used in modern es module codebases to designate the entry point for code that makes use of import/export instead of commonjs.
It's used by a lot more bundlers than Browserify, so I removed that specification.
8f9514a
to
66adad4
Compare
This PR has been largely ignored, so I'm just going to close it. |
@probablyup if you're still up for it, I'd love to have this land. Mind reopening and rebasing? |
@probablyup Any update on this? Will you reopen the PR? |
I don’t have time to work on this, sorry.
…On Tue, Jan 30, 2018 at 8:51 AM Emanuele Feliziani ***@***.***> wrote:
@probablyup <https://github.com/probablyup> Any update on this? Will you
reopen the PR?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2704 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAiy1s5GjL_FjDXWhcAXxKQwlDjN0rfcks5tPx5KgaJpZM4LuZ15>
.
|
For those following along: #5485 |
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. |
This is used in modern es module codebases to designate the entry point for code that makes use of import/export instead of commonjs. Jest doesn't need to care about whether or not that destination code is actually an ES module, just that the path gets resolved 🙃
Closes #2702