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

Moving npm-is into pwa-studio. #3106

Merged
merged 12 commits into from
Jun 11, 2021
Merged

Moving npm-is into pwa-studio. #3106

merged 12 commits into from
Jun 11, 2021

Conversation

revanth0212
Copy link
Contributor

@revanth0212 revanth0212 commented Apr 6, 2021

Description

npm-is a utility package that validates if a given script is executed using the given package manager. For instance, pwa-studio can only be installed using yarn and if you try to install using npm it has to fail. @zetlen created this wonderful utility package a while ago which we have been using in the preinstall script to check validate if yarn is being used to install the package. Unfortunately in npm 7 update a critical piece of code that the package relies on has changed. Specifically the value of process.env.npm_execpath when it is invoked using npx. The script extensively relies on the value of process.env.npm_execpath to make the decision about what package manager is used to initiating the script.

Previously (npm 6 or less) if you initiated a script using yarn which internally uses npx the value of process.env.npm_execpath was PATH_TO_THE_YARN_EXECUTABLE but after the update, the value of process.env.npm_execpath has changed to PATH_TO_THE_NPX_EXECUTABLE instead. Due to this the validation has failed and pwa-studio installation fails with the following error:

image

This PR addresses this issue by removing all references of npm-is and adding a note in the documentation to avoid using npm.

Related Issue

Closes PWA-1660

Acceptance

Should be able to validate if a script is initiated using a given package manager.

Verification Stakeholders

@zetlen
@dpatil-magento

Verification Steps

  1. Checkout the branch.
  2. Run yarn install and it should work fine.
  3. Run npm install and it should fail.

Checklist

  • I have added tests to cover my changes, if necessary.
  • I have added translations for new strings, if necessary.
  • I have updated the documentation accordingly, if necessary.

@revanth0212 revanth0212 added the version: Minor This changeset includes functionality added in a backwards compatible manner. label Apr 6, 2021
packages/npm-is/README.md Outdated Show resolved Hide resolved
@PWAStudioBot
Copy link
Contributor

PWAStudioBot commented Apr 6, 2021

Messages
📖

Associated JIRA tickets: PWA-1660.

📖 DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).
📖

Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.

Generated by 🚫 dangerJS against 40b8fc5

@sirugh sirugh added the needs-triage A pull request or issue that needs to be triaged prior to being synced to JIRA label May 28, 2021
@sirugh
Copy link
Contributor

sirugh commented Jun 4, 2021

@revanth0212 what's up with this PR?

@anthoula anthoula removed the needs-triage A pull request or issue that needs to be triaged prior to being synced to JIRA label Jun 7, 2021
@revanth0212 revanth0212 requested a review from jcalcaben June 7, 2021 21:33
package.json Outdated Show resolved Hide resolved
sirugh
sirugh previously approved these changes Jun 10, 2021
@supernova-at supernova-at merged commit d0f3d08 into develop Jun 11, 2021
@supernova-at supernova-at deleted the revanth/npm-is-fix branch June 11, 2021 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:npm-is Progress: done version: Minor This changeset includes functionality added in a backwards compatible manner.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants