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

Node warnings with more readable Node versions #410

Merged
merged 2 commits into from
Mar 1, 2022

Conversation

vmarta
Copy link
Contributor

@vmarta vmarta commented Feb 16, 2022

Description

Feedback from sprint demo:
The node version range is now taken from the engines field of the package.json, which is more readable than seeing semver's representation (>=12.0.0 <13.0.0-0||>=14.0.0 <15.0.0-0).

Follow up to this merged PR #384

Types of Changes

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Documentation update
  • Breaking change (could cause existing functionality to not work as expected)
  • Other changes (non-breaking changes that does not fit any of the above)

Breaking changes include:

  • Removing a public function or component or prop
  • Adding a required argument to a function
  • Changing the data type of a function parameter or return value
  • Adding a new peer dependency to package.json

How to Test-Drive This PR

First of all, run npm ci at the root of the monorepo to rebuild all the packages.

Then try testing the generator:

  1. Use an incompatible Node version: nvm use 13
  2. Generate a project with node packages/pwa-kit-create-app/scripts/create-mobify-app-dev.js --outputDir /tmp/project
  3. You should see a warning like this ... Your app may not work as expected when deployed to Managed Runtime servers which are compatible with Node ^12.0.0 || ^14.0.0

Also, try running the dev server from within the 'pwa' package:

  1. npm start
  2. Verify the node warning message

And try running npm ci at the root folder too with an incompatible node version.

Checklists

General

  • Changes are covered by test cases
  • CHANGELOG.md updated with a short description of changes (not required for documentation updates)

Accessibility Compliance

You must check off all items in one of the follow two lists:

  • There are no changes to UI

or...

Localization

  • Changes include a UI text update in the Retail React App (which requires translation)

The node version range is now taken from the engines field of the package.json, which is more readable than seeing semver's representation (>=12.0.0 <13.0.0-0||>=14.0.0 <15.0.0-0).
@vmarta vmarta added the wip Work in process label Feb 16, 2022
@vmarta vmarta requested a review from a team as a code owner February 16, 2022 20:29
@vmarta vmarta added ready for review PR is ready to be reviewed and removed wip Work in process labels Feb 17, 2022
@vmarta vmarta changed the title Node warnings with more readable version formatting Node warnings with more readable Node versions Feb 23, 2022
Copy link
Contributor

@vcua-mobify vcua-mobify left a comment

Choose a reason for hiding this comment

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

+1 LGTM

@vmarta vmarta merged commit d396fd3 into develop Mar 1, 2022
@vmarta vmarta deleted the better-node-version-formatting branch March 1, 2022 18:36
adamraya added a commit that referenced this pull request Mar 22, 2022
* Fix SDK's peer dependencies versioning (#432)

Previously the range was matching all possible versions. Now matching React 16 and 17 only.

* Node warnings with better version formatting (#410)

The node version range is now taken from the engines field of the package.json, which is more readable than seeing semver's representation (>=12.0.0 <13.0.0-0||>=14.0.0 <15.0.0-0).

* [Circle CI] Add test generated project job on Windows (#412)

* Update config.yml

* Add `generatedSFCCProjectTestWindows` test job

* Test not using gtime.js script

* Increase log verbosity

* Run first the CI job generated project on Windows

* Use  outputDir generated-project

* Use execSync

* Update npx pwa-kit-create-app command

* Use process.argv as outputDir

* Test execFileSync

* Add try/catch to execSync

* Show npm i logging

* Show npm install logs as we do in the BFH branch

* Show verdaccio logs

* More logs npm install verbose

* Use Node child_process & Update verdaccio config & bump version

* Use verdaccio npmjs cache true

* Set GENERATOR_PRESET env variable

* Update create-mobify-app.js

* Test using a xlarge Windows machine

* Increase Verdaccio max_fails config

* Remove server keepAliveTimeout

* Dump all Verdaccio logs to a file for debugging

* Fix smoketestscripts on Windows

* Add Circle CI unit to the scripts path

* Really fix smoketestscripts Circle CI job on Windows

* Update config.yml

* More testing smoke test script on Windows

* And more testing smoke test script on Windows

* smoketestscripts job working on Windows

* fix typo

* Clean up

* More clean up

* Update latest v4 verdaccio

* Remove file from bad merge

Co-authored-by: Vincent Marta <vmarta@salesforce.com>
@adamraya adamraya mentioned this pull request Mar 22, 2022
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review PR is ready to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants