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

Move standard --format into a separate module #340

Closed
mafintosh opened this issue Nov 19, 2015 · 25 comments
Closed

Move standard --format into a separate module #340

mafintosh opened this issue Nov 19, 2015 · 25 comments

Comments

@mafintosh
Copy link
Contributor

Currently around ~1/3 of the dependencies are there to support standard --format. I propose we move that command to a separate module standard-format since it isn't related to linting and would make standard a lot "lighter" to install.

@bcomnes
Copy link
Member

bcomnes commented Nov 19, 2015

I am okay with this, and maybe revisit adding it back if we can get it down to a much smaller set of deps.

@bcomnes
Copy link
Member

bcomnes commented Nov 19, 2015

We should pull the editor plugin authors into this discussion too

cc @stephenkubovic thoughts?

@yoshuawuyts
Copy link
Contributor

yeah, +1 on not including standard-format by default. Making standard lighter would be a good thing. Perhaps make standard -F run standard-format if it's available? Perhaps that would just be more confusing so I'm not sure.

@stephenkubovic
Copy link
Contributor

+1. This should also avoid formatting issues being opened in the linter.

@watson
Copy link
Member

watson commented Nov 19, 2015

👍

@feross feross added the v6 label Nov 21, 2015
@feross
Copy link
Member

feross commented Nov 21, 2015

Here are some real-world measurements with npm3.

With standard-format built-in (two runs):

npm i  25.38s user 5.98s system 62% cpu 49.785 total
npm i  26.71s user 5.58s system 68% cpu 47.242 total

Without standard-format:

npm i  13.11s user 3.05s system 56% cpu 28.585 total
npm i  14.33s user 3.18s system 58% cpu 29.767 total

So it cuts install time in half. And it removes the following dependencies:

- [email protected] node_modules/abbrev
- [email protected] node_modules/acorn
- [email protected] node_modules/alter
- [email protected] node_modules/ast-traverse
- [email protected] node_modules/ast-types
- [email protected] node_modules/babel-core
- [email protected] node_modules/babel-core/node_modules/globals
- [email protected] node_modules/babel-core/node_modules/minimatch
- [email protected] node_modules/babel-core/node_modules/source-map
- [email protected] node_modules/babel-plugin-constant-folding
- [email protected] node_modules/babel-plugin-dead-code-elimination
- [email protected] node_modules/babel-plugin-eval
- [email protected] node_modules/babel-plugin-inline-environment-variables
- [email protected] node_modules/babel-plugin-jscript
- [email protected] node_modules/babel-plugin-member-expression-literals
- [email protected] node_modules/babel-plugin-property-literals
- [email protected] node_modules/babel-plugin-proto-to-assign
- [email protected] node_modules/babel-plugin-react-constant-elements
- [email protected] node_modules/babel-plugin-react-display-name
- [email protected] node_modules/babel-plugin-remove-console
- [email protected] node_modules/babel-plugin-remove-debugger
- [email protected] node_modules/babel-plugin-runtime
- [email protected] node_modules/babel-plugin-undeclared-variables-check
- [email protected] node_modules/babel-plugin-undefined-to-void
- [email protected] node_modules/babylon
- [email protected] node_modules/bluebird
- [email protected] node_modules/breakable
- [email protected] node_modules/commander
- [email protected] node_modules/commoner
- [email protected] node_modules/config-chain
- [email protected] node_modules/convert-source-map
- [email protected] node_modules/core-js
- [email protected] node_modules/defs
- [email protected] node_modules/defs/node_modules/esprima-fb
- [email protected] node_modules/defs/node_modules/window-size
- [email protected] node_modules/defs/node_modules/yargs
- [email protected] node_modules/detect-indent
- [email protected] node_modules/detective
- [email protected] node_modules/diff
- [email protected] node_modules/disparity
- [email protected] node_modules/esformatter
- [email protected] node_modules/esformatter-eol-last
- [email protected] node_modules/esformatter-ignore
- [email protected] node_modules/esformatter-jsx
- [email protected] node_modules/esformatter-literal-notation
- [email protected] node_modules/esformatter-literal-notation/node_modules/esprima
- [email protected] node_modules/esformatter-literal-notation/node_modules/rocambole
- [email protected] node_modules/esformatter-quotes
- [email protected] node_modules/esformatter-semicolon-first
- [email protected] node_modules/esformatter-spaced-lined-comment
- [email protected] node_modules/esformatter/node_modules/debug
- [email protected] node_modules/esformatter/node_modules/strip-json-comments
- [email protected] node_modules/esformatter/node_modules/supports-color
- [email protected] node_modules/extend
- [email protected] node_modules/fresh-falafel
- [email protected] node_modules/fs-readdir-recursive
- [email protected] node_modules/graceful-readlink
- [email protected] node_modules/home-or-tmp
- [email protected] node_modules/home-or-tmp/node_modules/user-home
- [email protected] node_modules/iconv-lite
- [email protected] node_modules/ini
- [email protected] node_modules/invert-kv
- [email protected] node_modules/is-absolute
- [email protected] node_modules/is-finite
- [email protected] node_modules/is-integer
- [email protected] node_modules/is-relative
- [email protected] node_modules/js-beautify
- [email protected] node_modules/js-tokens
- [email protected] node_modules/jsesc
- [email protected] node_modules/json5
- [email protected] node_modules/lcid
- [email protected] node_modules/left-pad
- [email protected] node_modules/leven
- [email protected] node_modules/line-numbers
- [email protected] node_modules/mout
- [email protected] node_modules/nopt
- [email protected] node_modules/npm-path
- [email protected] node_modules/npm-run
- [email protected] node_modules/os-locale
- [email protected] node_modules/os-tmpdir
- [email protected] node_modules/output-file-sync
- [email protected] node_modules/path-exists
- [email protected] node_modules/private
- [email protected] node_modules/proto-list
- [email protected] node_modules/protochain
- [email protected] node_modules/q
- [email protected] node_modules/recast
- [email protected] node_modules/recast/node_modules/esprima-fb
- [email protected] node_modules/recast/node_modules/source-map
- [email protected] node_modules/regenerate
- [email protected] node_modules/regenerator
- [email protected] node_modules/regenerator/node_modules/esprima-fb
- [email protected] node_modules/regexpu
- [email protected] node_modules/regexpu/node_modules/esprima
- [email protected] node_modules/regjsgen
- [email protected] node_modules/regjsparser
- [email protected] node_modules/repeating
- [email protected] node_modules/resolve
- [email protected] node_modules/rocambole
- [email protected] node_modules/rocambole-indent
- [email protected] node_modules/rocambole-linebreak
- [email protected] node_modules/rocambole-node
- [email protected] node_modules/rocambole-token
- [email protected] node_modules/rocambole-whitespace
- [email protected] node_modules/rocambole/node_modules/esprima
- [email protected] node_modules/semver
- [email protected] node_modules/serializerr
- [email protected] node_modules/shebang-regex
- [email protected] node_modules/simple-fmt
- [email protected] node_modules/simple-is
- [email protected] node_modules/slash
- [email protected] node_modules/source-map-support
- [email protected] node_modules/source-map-support/node_modules/source-map
- [email protected] node_modules/stable
- [email protected] node_modules/standard-format
- [email protected] node_modules/stdin
- [email protected] node_modules/string.prototype.endswith
- [email protected] node_modules/stringmap
- [email protected] node_modules/stringset
- [email protected] node_modules/sync-exec
- [email protected] node_modules/to-fast-properties
- [email protected] node_modules/trim-right
- [email protected] node_modules/try-resolve
- [email protected] node_modules/tryor
- [email protected] node_modules/which
- [email protected] node_modules/y18n

@feross
Copy link
Member

feross commented Nov 21, 2015

I'm okay with removing standard-format. This is a breaking change, so let's batch it with the other breaking changes for v6.0.0. I added a v6 tag for this.

@mafintosh
Copy link
Contributor Author

\o/

@bcomnes
Copy link
Member

bcomnes commented Nov 23, 2015

Also, standard-format is definitely looking for some help.

There are lots of ES6 issues that probably require modification of upstream deps, and some attention to reducing the number of dependencies that it pulls in would also improve things a lot. It works great for the tried and true ES5 style JS, but you end up having to work around it for ES6 and react stuff.

I'll try to clean up the issues over the next week and document ways people can help out.

@Flet
Copy link
Member

Flet commented Nov 23, 2015

👍

I like @yoshuawuyts idea of running standard-format if its available when using -F. especially since its been available for so long.

@jzaefferer
Copy link

You can print an error otherwise, something like "standard-format is no longer bundled with standard, you need to install it separately"

@bcomnes
Copy link
Member

bcomnes commented Nov 23, 2015

Never done anything like that before, but I like the idea, with the fallback to a message like @jzaefferer mentioned.

@feross
Copy link
Member

feross commented Dec 2, 2015

Maybe we can do a standard-format hackathon and try to fix most of the open issues. standard-format is a big reason why lots of people like to use standard.

@Flet
Copy link
Member

Flet commented Dec 10, 2015

Yes, a hackathon for getting standard-format patched up would be awesome! It may also turn into an esformatter hackathon really... but maybe @millermedeiros would not mind this? :)

@tjmehta
Copy link

tjmehta commented Dec 22, 2015

@feross lmk about a standard-format fixes hackathon. I would love to lend a hand.

mafintosh added a commit to mafintosh/standard that referenced this issue Feb 1, 2016
@feross feross mentioned this issue Feb 4, 2016
25 tasks
@feross
Copy link
Member

feross commented Feb 4, 2016

This is completed now, and in the v6 branch.

@feross feross closed this as completed Feb 4, 2016
feross pushed a commit that referenced this issue Feb 4, 2016
@tunnckoCore
Copy link

Why not just moved it to devDependencies and save the --format option, i think it would have same effect? I'm just curious, i have both of them installed globally either way.

@watson
Copy link
Member

watson commented Feb 6, 2016

@tunnckoCore devDependencies are not installed when installing a module using npm install <module> - neither globally or locally. They are only installed when running npm link or npm install from the root of the module having the devDependencies.

@tunnckoCore
Copy link

@watson I know that. And exactly because of that, this will throw to the users and they will install it if they want/use formating.
I like to use this technic for optional things. For example in request-all package and delegate-properties.
I think that's the better way instead of dropping support at all, worth nothing and saves functionality.

@junosuarez
Copy link

My CI builds thank you 🙌

@feross
Copy link
Member

feross commented Feb 8, 2016

@jden 🚀

@dcousens
Copy link
Member

dcousens commented Feb 9, 2016

I think keeping standard --format with an error saying "install X package" is a sane deprecating option, and even long term.

@bcomnes
Copy link
Member

bcomnes commented Feb 9, 2016

I think keeping standard --format with an error saying "install X package" is a sane deprecating option, and even long term.

SGTM.

@tunnckoCore
Copy link

👍

@feross
Copy link
Member

feross commented Feb 9, 2016

@dcousens @bcomnes @tunnckoCore This change is already shipped in standard v6.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Archived in project
Development

No branches or pull requests