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

Add note about using Node 12 #1207

Merged

Conversation

nichhk
Copy link
Member

@nichhk nichhk commented May 25, 2022

Fixes #1205

  • Up to date with dev branch
  • Branch name follows guidelines
  • All PR Status checks are successful
  • Peer reviewed and approved

Any questions? See the getting started guide

@nichhk nichhk linked an issue May 25, 2022 that may be closed by this pull request
@nichhk nichhk requested a review from ardada2468 May 25, 2022 21:26
Copy link
Contributor

@ardada2468 ardada2468 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got a couple of npm errors using npm 12

  • It had to do with the post install script in package.json

  • "postinstall": "sh -c \"are-you-es5 check -r . | tail -n 2 | head -n 1 > ./node_modules_non_es5 \""

  • This might might work on unix based machines, but it will not work on windows.

  • As a temporary fix I just replaced the string above with a empty string and it was able to install all packages and load up the development server locally.

  • I think we need to create a new issue for this, and also edit the the documentations temporarily for windows users, and instruct them to replace the following line to empty string

  • This is the error I got
    NPM Error.txt

  • This is the package.json file that got rid of the errors.
    PackageJson.txt

@nichhk
Copy link
Member Author

nichhk commented May 25, 2022

Thanks for taking a look Arnav! Good to know about the issue with the postinstall script. I dug into it a bit, and it looks like this was used to identify some issues with the site on IE11. It seems like the postinstall was just used once to update some dependencies.

So I think we can safely delete the postinstall line entirely. But I agree that that change should be tracked in a separate issue. Can you open the issue please? Is there anything blocking this PR?

@ardada2468 ardada2468 merged commit 327b6c7 into dev May 25, 2022
@ardada2468 ardada2468 deleted the 1205-update-clientreadmemd-to-tell-devs-to-use-node-12 branch May 25, 2022 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update client/README.md to tell devs to use Node 12
2 participants