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

[eslint-plugin-specs] fix npm lifecycle hook #34273

Closed
wants to merge 3 commits into from

Conversation

kelset
Copy link
Contributor

@kelset kelset commented Jul 26, 2022

Summary

In 0.70-stable we started seeing yarn run lint fail; turns out, this is happening because the latest release of @react-native/eslint-plugin-specs 0.0.4 (from ea8d8e2) is faulty: the underlying reason is that since 0.0.2 there was this local workaround in place 2d06e6a (that changes a flag from false to true) but in NPM 8 the lifecycle hook prepublish has been deprecated so this pre publishing script to change the flag was not run (you can easily verify by checking the node_module and see react-native-modules.js the flag on L17 is set to false).

This PR addresses it by moving to the new lifecycle hook prepack (and modifies the other files accordingly).

After this PR is merged, we'll need to cherry pick it into 0.70 and do both a new release from the 0.70 branch and one from the main branch to have new versions of this module with the flag set correctly.

Changelog

[General] [Fixed] - Fix eslint-plugin-specs prepack npm lifecycle hook now that we use npm 8

Test Plan

Go in the eslint-plugin-specs folder, run npm pack, unzip the generated tar file, go into package, verify that L17 of react-native-modules.js is correctly changed to const PACKAGE_USAGE = true; (instead of false - which is what happen without this fix).

@kelset kelset requested review from lunaleaps and dmytrorykun July 26, 2022 11:12
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Contributor A React Native contributor. p: Microsoft Partner: Microsoft Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Jul 26, 2022
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 743f9ff
Branch: main

@facebook-github-bot
Copy link
Contributor

@dmitryrykun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @kelset in 8441c4a.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Jul 26, 2022
@kelset kelset deleted the kelset/fix-eslint-plugin-specs-prepublish branch July 26, 2022 14:26
dmytrorykun pushed a commit that referenced this pull request Jul 26, 2022
Summary:
In `0.70-stable` we started seeing `yarn run lint` fail; turns out, this is happening because the latest release of `react-native/eslint-plugin-specs` 0.0.4 (from ea8d8e2) is faulty: the underlying reason is that since 0.0.2 there was this local workaround in place 2d06e6a (that changes a flag from false to true) but in NPM 8 the lifecycle hook `prepublish` has [been deprecated](https://docs.npmjs.com/cli/v8/using-npm/scripts#life-cycle-scripts) so this pre publishing script to change the flag was not run (you can easily verify by checking the node_module and see `react-native-modules.js` the flag on L17 is set to false).

This PR addresses it by moving to the new lifecycle hook `prepack` (and modifies the other files accordingly).

After this PR is merged, we'll need to cherry pick it into 0.70 and do both a new release from the 0.70 branch and one from the main branch to have new versions of this module with the flag set correctly.

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[General] [Fixed] - Fix eslint-plugin-specs prepack npm lifecycle hook now that we use npm 8

Pull Request resolved: #34273

Test Plan: Go in the `eslint-plugin-specs` folder, run `npm pack`, unzip the generated tar file, go into `package`, verify that L17 of `react-native-modules.js` is correctly changed to `const PACKAGE_USAGE = true;` (instead of false - which is what happen without this fix).

Reviewed By: GijsWeterings

Differential Revision: D38151433

Pulled By: dmitryrykun

fbshipit-source-id: 7c4f13dae70eb731d57cbafa90f7be05c9bb8576
@cortinico
Copy link
Contributor

Thank you very much @kelset for investigating this and fixing this issue 🙏 We got affected by a similar issue in #34198

I've checked through the codebase and haven't found other occurrences of prepublish so we should be good to go for the future 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Contributor A React Native contributor. Merged This PR has been merged. p: Microsoft Partner: Microsoft Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants