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

[DOCS] npm scripts update #729

Closed
wants to merge 2 commits into from

Conversation

mikemimik
Copy link
Contributor

What / Why

This is a continuation of the work that @seanhealy has done. The docs were changed to gatsby with a new styling and the markdown was not rendering correctly. I've simply updated some of the formatting and condensed some sections of information together.

References

@mikemimik mikemimik added semver:patch semver patch level for changes Enhancement new feature or improvement Release 6.x work is associated with a specific npm 6 release Community labels Jan 27, 2020
@mikemimik mikemimik added this to the OSS - Sprint 2 milestone Jan 27, 2020
@mikemimik mikemimik requested a review from a team January 27, 2020 21:08
@mikemimik mikemimik self-assigned this Jan 27, 2020
mikemimik pushed a commit that referenced this pull request Jan 27, 2020
- A continuation of @seanhealy's work

PR-URL: #729
Credit: @mikemimik
Close: #729
Reviewed-by: @mikemimik
that is not dependent on the operating system or architecture of the
target system, use a `prepublish` script. This includes
tasks such as:
The `"scripts"` property of of your `package.json` file supports a number of built-in scripts and their preset life cycle events as well as arbitrary scripts. These all can be executed by running `npm run-script <stage>` or `npm run <stage>` for short. *Pre* and *post* commands with matching names will be run for those as well (e.g. `premyscript`, `myscript`, `postmyscript`). Scripts from dependencies can be run with `npm explore <pkg> -- npm run <stage>`.
Copy link
Contributor

Choose a reason for hiding this comment

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

it might be worth noting that although "test" can be user-defined/overridden, "install" can't be (and probably others, like "publish" etc)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a great point, same with pack. I'll add a "caveats" section. Does that sound right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rather than a caveats section I think what I mean is that we have some shorthands for scripts.

npm run test -> npm test
npm run start -> npm start
npm run build -> npm build
npm run restart -> npm restart

I can't think of any others, can you @ljharb ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Shorthands that can be overridden for user scripts would also include version, but I can't think of any others.

It'd be great to exhaustively list BOTH sets - ie, one list of "shorthand commands that run user scripts when present" and "shorthand commands that ignore user scripts even if present" (noting that they all invoke pre/post scripts)

There are some special life cycle scripts that happen only in certain situations. These scripts happen in addtion to the "pre" and "post" script.
* `prepare`, `prepublish`, `prepublishOnly`, `prepack`, `postpack`

**prepare**
Copy link
Contributor

Choose a reason for hiding this comment

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

it would be helpful to indicate the npm version in which each of these scripts was added inline (not just in the paragraph below, and also including prepack/postpack?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that's a great idea, that context is missing from this update. prepack and postpack are included in the list. Did you mean add the version context to those two script names as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems only the version information about prepare was there before. I've included it in here now :D

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, i have no idea when prepack/postpack/pack was added :-p

@mikemimik mikemimik force-pushed the mikemimik/feature/npm-scripts-update branch from 83f2241 to e95987c Compare January 28, 2020 00:01
@npm-deploy-user
Copy link

npm-deploy-user commented Jan 28, 2020

angular-quickstart app-large app-medium ember-quickstart react-app
prev current status prev current status prev current status prev current status prev current status
initial install 41s 39.5s 39.5s 35.5s 34.5s 33.2s 28.1s 25.7s 33.5s 30.4s
repeat install 9.6s 8.2s 8.6s 7.7s 8.3s 7.7s 7.7s 6.6s 9.1s 7.7s
with warm cache 32.3s 28.3s 33.8s 30.5s 31.3s 26.3s 23.4s 20.7s 29.1s 26.9s
with node_modules 9.2s 8.2s 8.6s 7.3s 9s 7.7s 7.8s 6.8s 9.4s 7.8s
with lockfile 32.6s 26.6s 31s 29.8s 29s 25.3s 21.7s 19.3s 27.4s 24.8s
with warm cache and node_modules 9.3s 8.6s 7.9s 7.2s 8.4s 8.1s 7.6s 7.4s 9.2s 8s
with warm cache and lockfile 24.7s 22.5s 26.3s 24.6s 24.3s 22.2s 17.8s 15.2s 21.3s 19.5s
with node_modules and lockfile 10.1s 8.3s 9.8s 7.6s 8.6s 8.1s 7.8s 6.8s 9.7s 8.5s

@mikemimik
Copy link
Contributor Author

Note: will be moving this from Sprint 2 -> Sprint 3 and rolled into the next release.

mikemimik pushed a commit that referenced this pull request Jan 28, 2020
- A continuation of @seanhealy's work

PR-URL: #729
Credit: @mikemimik
Close: #729
Reviewed-by: @mikemimik

### Life Cycle Scripts

There are some special life cycle scripts that happen only in certain situations. These scripts happen in addtion to the "pre" and "post" script.
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo in 'addtion'.

@claudiahdz
Copy link
Contributor

Can we also improve wording on the "Default Values" section? Just to make it clearer users don't have to explicitly declare a start script in the case of having a sever.js file, for example.

@darcyclarke
Copy link
Contributor

Closed out by @mikemimik in #729

@claudiahdz claudiahdz deleted the mikemimik/feature/npm-scripts-update branch March 30, 2020 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement new feature or improvement Release 6.x work is associated with a specific npm 6 release semver:patch semver patch level for changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants