Conversation
alexander-akait
force-pushed
the
fix-package-json
branch
from
7d57444 to
7b9a7ab
Compare
| @@ -1,13 +1,12 @@ | |||
| import cls from "classnames"; | |||
| import s from "./Button.css"; | |||
| import * as s from "./Button.css"; | |||
There was a problem hiding this comment.
By default all classnames in named export (for better tree shaking)
| const classes = cls(className, { | ||
| [s.button]: true, | ||
| [s.active]: active, | ||
| [s.toggle]: toggle, |
There was a problem hiding this comment.
We don't have toggle class
| onFocus={this.handleFocus} | ||
| /> | ||
| {this.state.showOptions ? ( | ||
| <div className={s.options}> |
There was a problem hiding this comment.
We don't have options class in CSS
| import escapeRegExp from "escape-string-regexp"; | ||
| import { escape } from "html-escaper"; | ||
| import filesize from "filesize"; | ||
| import { filesize } from "filesize"; |
There was a problem hiding this comment.
Now we need to use named export
| highlightedModules: computed, | ||
| foundModulesInfo: computed | ||
| }); | ||
| } |
There was a problem hiding this comment.
Avoid using decorators (they are deprecated in mobx too)
|
|
||
| :export { | ||
| toggleTime: 200ms; | ||
| } |
There was a problem hiding this comment.
We are using this in JS, so let's export this value using ICSS :export
| hasConcatenatedModules: computed, | ||
| foundModulesSize: computed, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Avoid using decorators (they are deprecated in mobx too)
| const { resolve, dirname } = require("path"); | ||
|
|
||
| const commander = require("commander"); | ||
| const { program: commanderProgram } = require("commander"); |
There was a problem hiding this comment.
commander now uses named export and ask to provide argument using argument()
| @@ -1,6 +1,5 @@ | |||
| const fs = require("fs"); | |||
| const path = require("path"); | |||
| const del = require("del"); | |||
There was a problem hiding this comment.
No need to use del, NodeJS has good fs module
| "devDependencies": { | ||
| "@babel/cli": "^7.28.6", | ||
| "@babel/core": "7.26.9", | ||
| "@babel/plugin-proposal-decorators": "7.25.9", |
There was a problem hiding this comment.
We don't need @babel/plugin-proposal-decorators anymore
| react: "preact/compat", | ||
| "react-dom/test-utils": "preact/test-utils", | ||
| "react-dom": "preact/compat", | ||
| mobx: require.resolve("mobx/lib/mobx.es6.js"), |
There was a problem hiding this comment.
No need, mobx by default provide es6 version
|
@valscion Ready for review, updated all deps |
valscion
left a comment
valscion
left a comment
There was a problem hiding this comment.
All in all looks good to me! Is there perhaps one stray console.log left over? Let's remove it if it was unnecessary and then it's OK to merge.
src/bin/analyzer.js
Outdated
| .parse(process.argv); | ||
| .parse(); | ||
|
|
||
| console.log(program.args); |
There was a problem hiding this comment.
Is this console.log here on purpose?
There was a problem hiding this comment.
@valscion My typo, forget when tests because we don't have tests for such things, removed, in future we will have no-console in eslint-config-webpack and will catch such things
|
@valscion ready to review again |
|
Thanks! |
2 participants
Summary
Here:
packageManagerversion to be align withengines.node, v20.9.0 include by defaultnpm@10.1.0^rangeWhat kind of change does this PR introduce?
chore
Did you add tests for your changes?
Existing
Does this PR introduce a breaking change?
No
If relevant, what needs to be documented once your changes are merged or what have you already documented?
Nothing