Skip to content

Switch to the only prepublish script#903

Merged
usulpro merged 5 commits intomasterfrom
shellJs_scripts
Apr 24, 2017
Merged

Switch to the only prepublish script#903
usulpro merged 5 commits intomasterfrom
shellJs_scripts

Conversation

@usulpro
Copy link
Copy Markdown
Member

@usulpro usulpro commented Apr 16, 2017

Issue: All packages use different versions of prepublish scripts. Plus some of them shell scripts which have bad compatibility with windows

What I did

  • Added the only instance of prepublish script to
    ./scripts/prepublish.js
    it's the shell js version wich allready was used in storybook-ui

  • Changed in scripts of each package.json to
    "prepublish": "node ../../scripts/prepublish.js"

  • Removed previos versions of prepublish scripts from packages

How to test

  1. clone this repo
  2. run yarn
    it should install all dependencies without any errors and buid dist's for all packages

Copy link
Copy Markdown
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Great change! See minor comment above.

Comment thread scripts/prepublish.js Outdated
const path = require('path');
const shell = require('shelljs');
const chalk = require('chalk');
const babel = ['node_modules', '.bin', 'babel'].join(path.sep);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use path.join?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

sure!

@usulpro
Copy link
Copy Markdown
Member Author

usulpro commented Apr 18, 2017

Thanks, @shilman!
I added --copy-files flag as well

@usulpro usulpro removed the request for review from ndelangen April 21, 2017 00:52
@usulpro
Copy link
Copy Markdown
Member Author

usulpro commented Apr 21, 2017

So if nobody objects I will merge this?

@ndelangen
Copy link
Copy Markdown
Member

@usulpro Are you working on getting this merged?

@usulpro
Copy link
Copy Markdown
Member Author

usulpro commented Apr 23, 2017

Yes @ndelangen I'll resolve it

usulpro added 5 commits April 24, 2017 17:33
use ./scripts/prepublish.js as a prepublish script for all packages
stories -> stories/ to ignore only folders
added test.js
@nx-cloud
Copy link
Copy Markdown

nx-cloud bot commented Apr 20, 2025

View your CI Pipeline Execution ↗ for commit 4845e21.

Command Status Duration Result
nx run-many -t build --parallel=3 ✅ Succeeded 1m 36s View ↗

☁️ Nx Cloud last updated this comment at 2025-04-20 07:45:50 UTC

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.

4 participants