-
-
Notifications
You must be signed in to change notification settings - Fork 618
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
test: ts esnext #2584
test: ts esnext #2584
Conversation
need to skip test for node < 14 |
Yep |
Codecov Report
@@ Coverage Diff @@
## master #2584 +/- ##
=======================================
Coverage 91.35% 91.35%
=======================================
Files 29 29
Lines 1480 1480
Branches 425 425
=======================================
Hits 1352 1352
Misses 128 128 Continue to review full report at Codecov.
|
hmm timing out here but seems to pass locally |
|
||
describe('webpack cli', () => { | ||
it('should support typescript esnext file', async () => { | ||
if (process.version.slice(1, 3) < 14) { |
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.
version in its own variable
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.
done
hm timing out on node 14 on macos 😞 |
Very weird, can you investigate? Maybe we really need increase timeout here |
Yep lets try increasing timeout, seems to work for me quickly in MacOS |
0806acd
test: rm eslint disable test: rm eslint disable
return; | ||
} | ||
|
||
if (isMacOS && !isWebpack5) { |
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.
Why here isWebpack5
? Happens only for v4 on macos?
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.
it's !isWebpack5
i.e 4 😄
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.
yes
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.
which is very weird as I don't know how webpack version change is causing this
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.
Doesn't work locally either 😞
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.
Doesn't work locally either disappointed
Do you mean it freezes on macos locally too?
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.
yep
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.
Very weird, bug we will drop webpack v4 in near future, so let's merge
|
||
const { exitCode, stderr, stdout } = await run(__dirname, ['-c', './webpack.config.ts'], { | ||
nodeOptions: ['--loader=ts-node/esm'], | ||
env: { WEBPACK_CLI_FORCE_LOAD_ESM_CONFIG: true }, |
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.
We should avoid using WEBPACK_CLI_FORCE_LOAD_ESM_CONFIG
here, it should works without it, locally fine
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.
yep lets avoid it
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.
done
What kind of change does this PR introduce?
test
Did you add tests for your changes?
Yes
If relevant, did you update the documentation?
NA
Summary
Add test for ts esm
Does this PR introduce a breaking change?
no
Other information
Fix #2458