[email protected] npm artifact contains dependency "temp" which was removed

[trivikr]

Describe the bug

The temp library was replaced by tmp in #633, and published in https://github.com/facebook/jscodeshift/releases/tag/v17.1.0.

However, the published artifact for v17.1.0 still calls temp, and it breaks in consumers.

Steps to reproduce

$ test-jscodeshift> npm install [email protected] --save-exact

$ test-jscodeshift> npx jscodeshift --version                  
jscodeshift: 17.1.0
 - babel: 7.26.0
 - babylon: 7.26.2
 - flow: 0.251.1
 - recast: 0.23.9

$ test-jscodeshift> grep -rnI 'temp' node_modules/jscodeshift/dist/Runner.js
node_modules/jscodeshift/dist/Runner.js:17:const temp = require('temp');
node_modules/jscodeshift/dist/Runner.js:193:            temp.open({ prefix: 'jscodeshift', suffix: ext }, (err, info) => {
node_modules/jscodeshift/dist/Runner.js:314:            temp.cleanupSync();

Observed behavior

The temp is still imported in published artifacts

Expected behavior

The tmp should be imported in published artifacts instead

Additional context

Bug report in consumer aws/aws-sdk-js-codemod#956

[alexbit-codemod]

@r4zendev fyi.

[trivikr]

Probably the build step was missed prior to manual publish, because of which npm artifact still imports temp dependency?
A quick fix would be to release (say v17.1.1) with the accurate artifact.

For future releases, we should prioritize moving to automation for doing releases like using changesets #625

[Daniel15]

I ran the build step, so this doesn't really make sense to me:

17:48 danlo@danlo-fedora-PW03KN9V ~/src/jscodeshift  (main)
% yarn prepare      
yarn run v1.22.22
$ cp -R src/ dist/
Done in 0.15s.

17:48 danlo@danlo-fedora-PW03KN9V ~/src/jscodeshift  (main)
% grep -r "'temp'" *
dist/Runner.js:const temp = require('temp');

Why did it not update dist/Runner.js?? I confirmed that src/Runner.js is correct. I always run the tests before publishing, and they all passed, but I guess those use src instead of dest...

I'll push 17.1.1 to fix this. I'll also update the prepare script to delete the dist directory first.

For future releases, we should prioritize moving to automation for doing releases like using changesets

Will do. I only just got approval to install the GitHub app yesterday, and haven't had a chance to fully look at all the other things we need to do to enable it.

[Daniel15]

Pushed 17.1.1 to resolve this. Manually verified that no more references to the temp module are present in the dist code.

@trivikr In addition to using changesets, I think we also need to add a smoke test that runs before a release can be published: Build the module, create a tarball, install it in a temp directory, run a basic codemod, ensure it works.

[ElonVolo]

I use np for publishing npm modules because it dots all the i's and crosses t's and redundantly makes sure important steps build/publish steps don't get left out.

Just my 2 cents.