Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
package.json
Outdated
| "@babel/eslint-parser": "^7.24.7", | ||
| "eslint": "8.56.0", | ||
| "jest": "^29.7.0", | ||
| "jest-environment-node-single-context": "^29.4.0", |
There was a problem hiding this comment.
What does this do? Removing isolation between tests seems dangerous. Why does it have to be used?
There was a problem hiding this comment.
This mitigates the error that appears when running yarn test. The fs-temp's utility chain of imports leads to a package named to-data-view which performs the following check:
Somehow, the data gets transformed into a Uint8Array and gets passed to this function, where it yields false for these checks ONLY in jest. The console.log check, of course, confirmed that the data was indeed an instance of Uint8Array, but the check result was still false. Spending some time on Github issues, I've found a suggestion that this is caused by jest and node having different contexts during the test suite run. This testing environment dependency helped to solve this issue.
I've tried running the same test with the same function that uses fs-temp to create temporary files in a testing project of my own and it was not happening, but in jscodeshift codebase it somehow did. So, instead of trying to update jest and/or write a custom environment file, I used this existing package.
There was a problem hiding this comment.
You know what, I removed the fs-temp dependency too and instead used 0-dep tmp module. Let me know if the new implementation needs improvements.
e8387f6 to
7f88065
Compare
temp library with tmp
|
Thanks! This is looking pretty good. I like 0-dependency libraries. Looked through the code of the module and it seems reasonable to me. Can you please call the Also, are you able to test on Windows to ensure it works properly? I see some Windows-specific code in the module: https://github.com/raszi/node-tmp/blob/5f0b2525ed6f6a977ea0cc272d4903d9d2216059/lib/tmp.js#L32. Let me know if you don't have a Windows system and I can test it (but I can't test MacOS - I've only got Linux and Windows systems). |
|
Looks good. Thanks! |
|
When is this change expected to be published? Will it be in |
|
Released as v17.1.0. I usually wait for more changes to bundle them all together into one release, to avoid a lot of churn. People don't like seeing updates all the time. That's why it took a while to release. We didn't end up with any other major changes in this release though. |
|
Looks like there was some issue with publishing v17.1.0 which includes removed dependency I've created a bug report at #638 |
This PR would fix the deprecation warnings that appear during the installation of
jscodeshift's peer dependencies.An example of such warnings can be reproduced by installing the
jscodeshiftCLI tool globally (npm i -g jscodeshift) or by installing any other package that has jscodeshift as a dependency. For instance, this is what the error looks like in my shell:The testing of this PR was done by performing the following steps:
yarn packnpm i -g ./path/to/tarball.tgzThe results are as shown:
After this PR is merged, a release will be required to fix the error for all of the dependants.