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

Comments

@mafintosh
Copy link
Contributor

commented Nov 19, 2015

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Member

commented Nov 19, 2015

We should pull the editor plugin authors into this discussion too

cc @stephenkubovic thoughts?

@yoshuawuyts

This comment has been minimized.

Copy link
Contributor

commented Nov 19, 2015

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

This comment has been minimized.

Copy link
Contributor

commented Nov 19, 2015

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

@watson

This comment has been minimized.

Copy link
Member

commented Nov 19, 2015

👍

@feross feross added the v6 label Nov 21, 2015

@feross

This comment has been minimized.

Copy link
Member

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:

- abbrev@1.0.7 node_modules/abbrev
- acorn@1.2.2 node_modules/acorn
- alter@0.2.0 node_modules/alter
- ast-traverse@0.1.1 node_modules/ast-traverse
- ast-types@0.8.12 node_modules/ast-types
- babel-core@5.8.34 node_modules/babel-core
- globals@6.4.1 node_modules/babel-core/node_modules/globals
- minimatch@2.0.10 node_modules/babel-core/node_modules/minimatch
- source-map@0.5.3 node_modules/babel-core/node_modules/source-map
- babel-plugin-constant-folding@1.0.1 node_modules/babel-plugin-constant-folding
- babel-plugin-dead-code-elimination@1.0.2 node_modules/babel-plugin-dead-code-elimination
- babel-plugin-eval@1.0.1 node_modules/babel-plugin-eval
- babel-plugin-inline-environment-variables@1.0.1 node_modules/babel-plugin-inline-environment-variables
- babel-plugin-jscript@1.0.4 node_modules/babel-plugin-jscript
- babel-plugin-member-expression-literals@1.0.1 node_modules/babel-plugin-member-expression-literals
- babel-plugin-property-literals@1.0.1 node_modules/babel-plugin-property-literals
- babel-plugin-proto-to-assign@1.0.4 node_modules/babel-plugin-proto-to-assign
- babel-plugin-react-constant-elements@1.0.3 node_modules/babel-plugin-react-constant-elements
- babel-plugin-react-display-name@1.0.3 node_modules/babel-plugin-react-display-name
- babel-plugin-remove-console@1.0.1 node_modules/babel-plugin-remove-console
- babel-plugin-remove-debugger@1.0.1 node_modules/babel-plugin-remove-debugger
- babel-plugin-runtime@1.0.7 node_modules/babel-plugin-runtime
- babel-plugin-undeclared-variables-check@1.0.2 node_modules/babel-plugin-undeclared-variables-check
- babel-plugin-undefined-to-void@1.1.6 node_modules/babel-plugin-undefined-to-void
- babylon@5.8.34 node_modules/babylon
- bluebird@2.10.2 node_modules/bluebird
- breakable@1.0.0 node_modules/breakable
- commander@2.9.0 node_modules/commander
- commoner@0.10.4 node_modules/commoner
- config-chain@1.1.9 node_modules/config-chain
- convert-source-map@1.1.2 node_modules/convert-source-map
- core-js@1.2.6 node_modules/core-js
- defs@1.1.1 node_modules/defs
- esprima-fb@15001.1001.0-dev-harmony-fb node_modules/defs/node_modules/esprima-fb
- window-size@0.1.4 node_modules/defs/node_modules/window-size
- yargs@3.27.0 node_modules/defs/node_modules/yargs
- detect-indent@3.0.1 node_modules/detect-indent
- detective@4.3.1 node_modules/detective
- diff@1.4.0 node_modules/diff
- disparity@2.0.0 node_modules/disparity
- esformatter@0.8.1 node_modules/esformatter
- esformatter-eol-last@1.0.0 node_modules/esformatter-eol-last
- esformatter-ignore@0.1.3 node_modules/esformatter-ignore
- esformatter-jsx@2.3.11 node_modules/esformatter-jsx
- esformatter-literal-notation@1.0.1 node_modules/esformatter-literal-notation
- esprima@1.0.4 node_modules/esformatter-literal-notation/node_modules/esprima
- rocambole@0.3.6 node_modules/esformatter-literal-notation/node_modules/rocambole
- esformatter-quotes@1.0.3 node_modules/esformatter-quotes
- esformatter-semicolon-first@1.1.0 node_modules/esformatter-semicolon-first
- esformatter-spaced-lined-comment@2.0.1 node_modules/esformatter-spaced-lined-comment
- debug@0.7.4 node_modules/esformatter/node_modules/debug
- strip-json-comments@0.1.3 node_modules/esformatter/node_modules/strip-json-comments
- supports-color@1.3.1 node_modules/esformatter/node_modules/supports-color
- extend@2.0.1 node_modules/extend
- fresh-falafel@1.2.0 node_modules/fresh-falafel
- fs-readdir-recursive@0.1.2 node_modules/fs-readdir-recursive
- graceful-readlink@1.0.1 node_modules/graceful-readlink
- home-or-tmp@1.0.0 node_modules/home-or-tmp
- user-home@1.1.1 node_modules/home-or-tmp/node_modules/user-home
- iconv-lite@0.4.13 node_modules/iconv-lite
- ini@1.3.4 node_modules/ini
- invert-kv@1.0.0 node_modules/invert-kv
- is-absolute@0.1.7 node_modules/is-absolute
- is-finite@1.0.1 node_modules/is-finite
- is-integer@1.0.6 node_modules/is-integer
- is-relative@0.1.3 node_modules/is-relative
- js-beautify@1.5.10 node_modules/js-beautify
- js-tokens@1.0.1 node_modules/js-tokens
- jsesc@0.5.0 node_modules/jsesc
- json5@0.4.0 node_modules/json5
- lcid@1.0.0 node_modules/lcid
- left-pad@0.0.3 node_modules/left-pad
- leven@1.0.2 node_modules/leven
- line-numbers@0.2.0 node_modules/line-numbers
- mout@0.11.1 node_modules/mout
- nopt@3.0.6 node_modules/nopt
- npm-path@1.0.2 node_modules/npm-path
- npm-run@1.1.1 node_modules/npm-run
- os-locale@1.4.0 node_modules/os-locale
- os-tmpdir@1.0.1 node_modules/os-tmpdir
- output-file-sync@1.1.1 node_modules/output-file-sync
- path-exists@1.0.0 node_modules/path-exists
- private@0.1.6 node_modules/private
- proto-list@1.2.4 node_modules/proto-list
- protochain@1.0.3 node_modules/protochain
- q@1.4.1 node_modules/q
- recast@0.10.33 node_modules/recast
- esprima-fb@15001.1001.0-dev-harmony-fb node_modules/recast/node_modules/esprima-fb
- source-map@0.5.3 node_modules/recast/node_modules/source-map
- regenerate@1.2.1 node_modules/regenerate
- regenerator@0.8.40 node_modules/regenerator
- esprima-fb@15001.1001.0-dev-harmony-fb node_modules/regenerator/node_modules/esprima-fb
- regexpu@1.3.0 node_modules/regexpu
- esprima@2.7.0 node_modules/regexpu/node_modules/esprima
- regjsgen@0.2.0 node_modules/regjsgen
- regjsparser@0.1.5 node_modules/regjsparser
- repeating@1.1.3 node_modules/repeating
- resolve@1.1.6 node_modules/resolve
- rocambole@0.7.0 node_modules/rocambole
- rocambole-indent@2.0.4 node_modules/rocambole-indent
- rocambole-linebreak@1.0.1 node_modules/rocambole-linebreak
- rocambole-node@1.0.0 node_modules/rocambole-node
- rocambole-token@1.2.1 node_modules/rocambole-token
- rocambole-whitespace@1.0.0 node_modules/rocambole-whitespace
- esprima@2.7.0 node_modules/rocambole/node_modules/esprima
- semver@4.3.6 node_modules/semver
- serializerr@1.0.2 node_modules/serializerr
- shebang-regex@1.0.0 node_modules/shebang-regex
- simple-fmt@0.1.0 node_modules/simple-fmt
- simple-is@0.2.0 node_modules/simple-is
- slash@1.0.0 node_modules/slash
- source-map-support@0.2.10 node_modules/source-map-support
- source-map@0.1.32 node_modules/source-map-support/node_modules/source-map
- stable@0.1.5 node_modules/stable
- standard-format@1.6.10 node_modules/standard-format
- stdin@0.0.1 node_modules/stdin
- string.prototype.endswith@0.2.0 node_modules/string.prototype.endswith
- stringmap@0.2.2 node_modules/stringmap
- stringset@0.2.1 node_modules/stringset
- sync-exec@0.5.0 node_modules/sync-exec
- to-fast-properties@1.0.1 node_modules/to-fast-properties
- trim-right@1.0.1 node_modules/trim-right
- try-resolve@1.0.1 node_modules/try-resolve
- tryor@0.1.2 node_modules/tryor
- which@1.2.0 node_modules/which
- y18n@3.2.0 node_modules/y18n
@feross

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Contributor Author

commented Nov 21, 2015

\o/

@bcomnes

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link

commented Nov 23, 2015

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

@bcomnes

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link

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 referenced this issue Feb 4, 2016

Closed

Release proposal: standard v6 #399

25 of 25 tasks complete
@feross

This comment has been minimized.

Copy link
Member

commented Feb 4, 2016

This is completed now, and in the v6 branch.

@feross feross closed this Feb 4, 2016

feross added a commit that referenced this issue Feb 4, 2016

@tunnckoCore

This comment has been minimized.

Copy link

commented Feb 6, 2016

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link

commented Feb 7, 2016

@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

This comment has been minimized.

Copy link

commented Feb 8, 2016

My CI builds thank you 🙌

@feross

This comment has been minimized.

Copy link
Member

commented Feb 8, 2016

@jden 🚀

@dcousens

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link

commented Feb 9, 2016

👍

@feross

This comment has been minimized.

Copy link
Member

commented Feb 9, 2016

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

@feross feross added enhancement and removed enhancement labels May 10, 2018

@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.
You can’t perform that action at this time.