Skip to content

Conversation

@alexander-akait
Copy link
Member

@alexander-akait alexander-akait commented Jan 21, 2026

Summary

Here:

  • unpin dependencies, we have a lock file, this doesn't make sense, it is a bad practice and it is simply useless since NPM uses a lock file for installation
  • fix the packageManager version to be align with engines.node, v20.9.0 include by default npm@10.1.0
  • update all deps regarding ^ range
  • update mobx
  • update commander and escapers
  • update webpack.config.js

What 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

@@ -1,13 +1,12 @@
import cls from "classnames";
import s from "./Button.css";
import * as s from "./Button.css";
Copy link
Member Author

Choose a reason for hiding this comment

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

By default all classnames in named export (for better tree shaking)

const classes = cls(className, {
[s.button]: true,
[s.active]: active,
[s.toggle]: toggle,
Copy link
Member Author

Choose a reason for hiding this comment

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

We don't have toggle class

onFocus={this.handleFocus}
/>
{this.state.showOptions ? (
<div className={s.options}>
Copy link
Member Author

Choose a reason for hiding this comment

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

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";
Copy link
Member Author

Choose a reason for hiding this comment

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

Now we need to use named export

highlightedModules: computed,
foundModulesInfo: computed
});
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Avoid using decorators (they are deprecated in mobx too)


:export {
toggleTime: 200ms;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

We are using this in JS, so let's export this value using ICSS :export

hasConcatenatedModules: computed,
foundModulesSize: computed,
});
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Avoid using decorators (they are deprecated in mobx too)

const { resolve, dirname } = require("path");

const commander = require("commander");
const { program: commanderProgram } = require("commander");
Copy link
Member Author

Choose a reason for hiding this comment

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

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");
Copy link
Member Author

Choose a reason for hiding this comment

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

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",
Copy link
Member Author

Choose a reason for hiding this comment

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

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"),
Copy link
Member Author

Choose a reason for hiding this comment

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

No need, mobx by default provide es6 version

@alexander-akait
Copy link
Member Author

@valscion Ready for review, updated all deps

Copy link
Member

@valscion valscion left a comment

Choose a reason for hiding this comment

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

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.

.parse(process.argv);
.parse();

console.log(program.args);
Copy link
Member

Choose a reason for hiding this comment

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

Is this console.log here on purpose?

Copy link
Member Author

Choose a reason for hiding this comment

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

@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

@alexander-akait
Copy link
Member Author

@valscion ready to review again

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.

3 participants