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

Cleanup package scripts #15497

Open
eps1lon opened this issue Apr 26, 2019 · 11 comments
Open

Cleanup package scripts #15497

eps1lon opened this issue Apr 26, 2019 · 11 comments
Labels
core Infrastructure work going on behind the scenes

Comments

@eps1lon
Copy link
Member

eps1lon commented Apr 26, 2019

We currently leverage the scripts entry in our package.json. As of right now this includes around 34 scripts that aren't really documented. Any addition, change etc. has to be followed by contributors to be understood.

It would be nice if we could consolidate those into a single place that also adds documentation. This should put emphasis on common tasks as well as guiding contributors: What script can I use to perform X task?

https://github.com/kentcdodds/nps looks very close to what I have in mind (task + description + watch mode in a single place).

Maybe there are other solutions for large monorepos. In any case it would be nice if we could improve the first contributor experience.

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 26, 2019

@eps1lon I wish we could write comments in a JSON file. It would make it clearer. Each npm run command fits for a specific use case. So far, we have been partially documenting the commands in /test/README.md and /CONTRIBUTING.md.

Do you have a specific pain point in mind, e.g. what changes do you refer to?

@cvanem
Copy link
Contributor

cvanem commented May 16, 2019

@oliviertassinari @eps1lon

Let me know if you need help or would like a PR to see what an nps setup looks like? There are a few options to consider:

  1. Leave scripts in package.json and point them to nps (i.e . argos: nps argos). The original yarn commands should still work as well as calling directly with nps.
  2. Remove scripts from package.json and user calls directly with nps.

Also, the nps init process replaces script name characters : and - with . and an uppercase, next occurring, letter (i.e. size-why => sizeWhy). The resulting scripts end up looking like below. There are ways around this, by wrapping the key's in quotes, but that seems like a dirty approach:

module.exports = {
  scripts: {
    argos: {
      script: 'argos upload test/regressions/screenshots/chrome --token $ARGOS_TOKEN',
      description: 'run the amazing argos upload script!!!',
    },
    docs: {
      api:
        'rimraf ./pages/api && cross-env BABEL_ENV=test babel-node ./docs/scripts/buildApi.js ./packages/material-ui/src ./pages/api && cross-env BABEL_ENV=test babel-node ./docs/scripts/buildApi.js ./packages/material-ui-lab/src ./pages/api',
      build: 'rimraf .next && cross-env NODE_ENV=production BABEL_ENV=docs-production next build',
      buildSw: 'babel-node ./docs/scripts/buildServiceWorker.js',
      deploy: 'git push material-ui-docs next:next',
      dev:
        'rimraf node_modules/.cache/babel-loader && cross-env BABEL_ENV=docs-development babel-node docs/src/server.js',
      export:
        'rimraf docs/export && next export -o docs/export && yarn docs:build-sw && cp -r docs/static/. docs/export',
      icons: 'rimraf static/icons/* && babel-node ./docs/scripts/buildIcons.js',
      sizeWhy: 'DOCS_STATS_ENABLED=true yarn docs:build',
      start: 'next start',
      i18N: 'cross-env BABEL_ENV=test babel-node ./docs/scripts/i18n.js',
      typescript: {
        default: 'node docs/scripts/formattedTSDemos --watch',
        check: 'tslint -p docs/tsconfig.json',
        formatted: 'node docs/scripts/formattedTSDemos',
      },
    },
...
}

All that said, nps provides added functionality but also adds an additional cli. I am not sure if this makes the process any easier for a first time contributor.

It would be nice if yarn supported a js filename input for the package.json scripts field as an alternative to the standard object hash of script commands. This would at least allow comments.

@eps1lon
Copy link
Member Author

eps1lon commented May 17, 2019

@cvanem This issue is totally up for grabs. This is what the "help wanted" issue is for.

The important part is that I don't want the create churn for maintainers. The old workflows shouldn't change. It's mainly directed at new contributors so that they don't feel overwhelmed when looking at the package.json and can have a single entry point.

Some points ordered by importance. They are up for debate starting with point 3 and can be implemented in separate PRs:

  1. document scripts
  2. improve readability and "workflow splitting"
    Essentially extract all && piping into separate scripts. For example when debugging docs generation I don't want to clutter my git diff by deleting all API docs.
  3. Watchmode all the things!
  4. Get rid of babel-node
    babel-node is for developing node scripts that are published. Since we only use those for internal usage we can require the latest active LTS node version. The only feature we loose is es modules and we don't need those for local dev. It also creates overhead when running the scripts because they have to be transpiled first.
  5. Document how to run the scripts with a debugger attached
    This is where no babel-node will help because we don't have to fight source maps.

nps isn't a requirement for this. From skimming the readme it just seemed like this would address 1 and 2.

@cvanem
Copy link
Contributor

cvanem commented May 18, 2019

@eps1lon

For removing babel node, can you take a look at this example PR and confirm this is what you are looking for?

Note the relative import paths. Also, the docs:api script links into the material-ui and material-ui-styles packages so I am not sure what the best path is for removing babel-node with that script. Can you advise?

@eps1lon
Copy link
Member Author

eps1lon commented May 20, 2019

Note the relative import paths. Also, the docs:api script links into the material-ui and material-ui-styles packages so I am not sure what the best path is for removing babel-node with that script. Can you advise?

I was wondering how much code is shared between client and server. Seems like we only need it for component source execution. Maybe we can switch to static analysis later. PR is a good starter though.

@cvanem
Copy link
Contributor

cvanem commented May 23, 2019

@eps1on

For items 1-3, I like the idea of using something like nps but I don't like the extra cli. I submitted a PR to yarn for importable scripts. We will see if it gains any traction.

@eps1lon
Copy link
Member Author

eps1lon commented May 24, 2019

For items 1-3, I like the idea of using something like nps but I don't like the extra cli.

I wouldn't expect to see much manual calling of nps. Just something like yarn help which gives an overview and then yarn some-script:help which prints help for this particular some-script

@cvanem
Copy link
Contributor

cvanem commented May 24, 2019

@eps1lon
You would still have to call nps scriptname every time you want to run a script right?
Every time you want to develop you would have to run nps docs:dev ?

I couldn't find a way to hide the nps cli without adding a bunch of aliases in the packages.json file. Is there a better way?

@eps1lon
Copy link
Member Author

eps1lon commented May 24, 2019

For new workflows yes but

The important part is that I don't want the create churn for maintainers.

stands above all. yarn docs:dev should still work as before. I would imagine docs:dev would just be an alias for yarn nps docs:dev.

@adeelibr
Copy link
Contributor

Can we instead just add a new section in our documentation & document what each script does along with all it's relevant information? 🤔 💭

@eps1lon
Copy link
Member Author

eps1lon commented May 29, 2019

Can we instead just add a new section in our documentation & document what each script does along with all it's relevant information?

It's part of the issue, yes. The problem is that documentation becomes easily outdated. Even putting it right next to the script won't prevent it. A separate file (or gh wikis) would be even worse.

Since I want to enhance the scripts anyway (+debug, +watch) it's a good opportunity to explore solutions that solve both issues (docs and better scripts).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes
Projects
None yet
Development

No branches or pull requests

5 participants