Update depedencies to fix known security vulnerabiltiies #165

malor · 2025-06-21T07:21:47Z

This quickly snowballs into updating the React version, as some of the dependent libraries require the latest one. Updating React requires dropping Enzyme in favour of React Testing Library.
The other major chunk of vulnerabilities is in webpack

Enzyme does not support React >= 17, and we want to upgrade to that version to resolve known security vulnerabilities in the dependant libraries. Apparentely, React Testing Library is the recommended replacement (https://testing-library.com/docs/react-testing-library/migrate-from-enzyme).

* This quickly snowballs to updating the React version as some of the dependant libraries (like react-tag-input) only support the latest one. * The other major chunk of vulnerabilities was in webpack. * Tests were migrated from Enzyme to React Testing Library beforehand

Apparantely, React Router 7 is going to bring some changes, and people are encouraged to opt int to those early.

malor · 2025-06-21T07:22:57Z

tests/testSetup.js

-// manipulate underlying UI library (it can be used not only with React).
-// Since we use React 16, we need to set a proper adapter.
-configure({ adapter: new Adapter() })
+// React Testing Library setup - jest-dom provides custom matchers for DOM elements


Looks like we don't need a custom test setup anymore?

Looks like it.

malor · 2025-06-21T07:23:13Z

webpack.config.js

@@ -3,7 +3,7 @@ const process = require('process')

 const webpack = require('webpack')

-const CleanWebpackPlugin = require('clean-webpack-plugin')
+const { CleanWebpackPlugin } = require('clean-webpack-plugin')


What is this change?

Unpacking, or deconstruction. Most likely require('clean-webpack-plugin') used to return just one class, and now it returns a mapping (object in js) where CleanWebpackPlugin is just one of its keys.

The syntax with { CleanWebpackPlugin } just allows to bind the CleanWebpackPlugin key of the require('clean-webpack-plugin') object to the CleanWebpackPlugin local constant.

malor · 2025-06-21T07:23:52Z

webpack.config.js

@@ -168,29 +168,24 @@ module.exports = () => {
              loader: 'css-loader',
              options: {
                modules: false,
-                minimize: true,


Why did Claude decide to drop this?

malor · 2025-06-21T08:23:26Z

webpack.config.js

                // Enable source maps if they are specified in devtool
                // option. God-Knows-Why css-loader doesn't check devtool
                // value in order to initialize its sourceMap value, hence
                // this line.
                sourceMap: true,
+                url: true,


Why is this needed now?

malor · 2025-06-21T08:23:58Z

webpack.config.js

        {
-          test: /\.(png|svg|jpg|woff|svg|ttf|woff2|eot)$/,


Why is svg dropped?

FWIW, it seems svg was duplicated.

malor · 2025-06-21T08:25:07Z

webpack.config.js

-
-    node: {
-      net: 'empty',
+      fallback: {


What's this for?

malor · 2025-06-21T08:26:10Z

src/styles/common/icons.styl

@@ -1,6 +1,6 @@
 @font-face
  font-family: "IconFont"
-  src: url("../assets/fonts/icon-font.woff") format("woff")
+  src: url("../../assets/fonts/icon-font.woff") format("woff")


It's only one level up?

malor · 2025-06-21T08:29:54Z

src/components/App.jsx

+              <Route path="/sign-in" element={<SignIn />} />
+              {/* Routes renders the first matched route, so we need to keep this
+                  one last as the regexp below matches the paths of other pages */}
+              <Route path="/:id" element={<Snippet />} />


([a-zA-Z0-9]+) should have been preserved?

Probably. But I don't think it's important. Worst case we will stop showing 404 on something that doesn't look like a legit snippet ID.

malor · 2025-06-21T08:38:46Z

package.json

  },
  "scripts": {
-    "lint": "eslint --ignore-path .gitignore --ext ts .",
+    "lint": "eslint --ext ts,tsx src/",


This no longer lints the tests?

malor · 2025-06-21T08:40:33Z

tests/components/common/Spinner.test.tsx


 import Spinner from '../../../src/components/common/Spinner'

+// Mock the SVG import to prevent test failures


What is this?

As far as I remember, webpack patches the import statement so it can track what was used and produce the final bundle that contains what was used only. In addition to that, it extends import behavior in not compatible way -- it allows importing assets, so it can do the same thing for icons and images and literally anything else.

malor · 2025-06-21T08:41:23Z

@lotrien @ikalnytskyi Check out this magnificent creation by Claude :)

ikalnytskyi · 2025-06-30T14:41:21Z

package.json

+    "joi": "^17.13.3",
+    "parse-link-header": "^2.0.0",
+    "querystring-es3": "^0.2.1",
+    "react": "^18.3.1",


Interesting, why it wasn't updated to the latest stable version? 🤔

ikalnytskyi · 2025-06-30T14:48:06Z

tests/components/common/Spinner.test.tsx


 import Spinner from '../../../src/components/common/Spinner'

+// Mock the SVG import to prevent test failures


As far as I remember, webpack patches the import statement so it can track what was used and produce the final bundle that contains what was used only. In addition to that, it extends import behavior in not compatible way -- it allows importing assets, so it can do the same thing for icons and images and literally anything else.

ikalnytskyi · 2025-06-30T14:49:41Z

tests/components/common/Spinner.test.tsx

+    const spinnerWrapper = container.querySelector('.spinner')
+
+    expect(spinnerWrapper).toBeInTheDocument()
+    expect(spinnerWrapper).toHaveClass('spinner')


nit: A silly check because we already query by a class name (see L15).

ikalnytskyi · 2025-06-30T14:52:12Z

tests/components/common/Title.test.tsx

+    const { container } = render(<Title title={title} />)
+    const titleElement = container.querySelector('.title')
+
+    expect(titleElement).toHaveClass('title')


ikalnytskyi · 2025-06-30T14:52:30Z

tests/testSetup.js

-// manipulate underlying UI library (it can be used not only with React).
-// Since we use React 16, we need to set a proper adapter.
-configure({ adapter: new Adapter() })
+// React Testing Library setup - jest-dom provides custom matchers for DOM elements


Looks like it.

ikalnytskyi · 2025-06-30T15:00:53Z

webpack.config.js


      // Each time we change something, a new version of bundled assets is
      // produced. Since we use hash in filenames in order to invalidate cache
      // on change, we end up having multiple outdated copies in output
      // directory. Let's cleanup it before produce a fresh build.
-      new CleanWebpackPlugin([path.resolve(__dirname, 'dist')]),


It seems it defaults now to output.path of the main webpack config which points to the same directory.

ikalnytskyi · 2025-06-30T15:11:23Z

src/styles/common/icons.styl

@@ -1,6 +1,6 @@
 @font-face
  font-family: "IconFont"
-  src: url("../assets/fonts/icon-font.woff") format("woff")
+  src: url("../../assets/fonts/icon-font.woff") format("woff")


ikalnytskyi · 2025-06-30T15:12:32Z

src/components/Sidebar.tsx

@@ -7,17 +7,17 @@ const Sidebar: FC = () => (
  <nav className="sidebar">
    <ul className="sidebar-list">
      <li className="sidebar-item">
-        <NavLink exact to="/" activeClassName="active" title="New Snippet">
+        <NavLink to="/" className={({ isActive }) => isActive ? "active" : ""} title="New Snippet" end>


Since our navlinks are always active, do we even need this change?

Also, what's that "end" at the end?

ikalnytskyi · 2025-06-30T15:14:23Z

src/components/App.jsx

+              <Route path="/sign-in" element={<SignIn />} />
+              {/* Routes renders the first matched route, so we need to keep this
+                  one last as the regexp below matches the paths of other pages */}
+              <Route path="/:id" element={<Snippet />} />


Probably. But I don't think it's important. Worst case we will stop showing 404 on something that doesn't look like a legit snippet ID.

ikalnytskyi · 2025-06-30T15:50:59Z

src/components/App.jsx

+              <Route path="/sign-in" element={<SignIn />} />
+              {/* Routes renders the first matched route, so we need to keep this
+                  one last as the regexp below matches the paths of other pages */}
+              <Route path="/:id" element={<Snippet />} />


Probably. But I don't think it's important. Worst case we will stop showing 404 on something that doesn't look like a legit snippet ID.

malor added 3 commits June 21, 2025 08:09

Fix test warnings

c2233d0

Apparantely, React Router 7 is going to bring some changes, and people are encouraged to opt int to those early.

malor commented Jun 21, 2025

View reviewed changes

malor requested review from ikalnytskyi and lotrien June 21, 2025 08:40

ikalnytskyi reviewed Jun 30, 2025

View reviewed changes


		import Spinner from '../../../src/components/common/Spinner'

		// Mock the SVG import to prevent test failures

Update depedencies to fix known security vulnerabiltiies #165

Are you sure you want to change the base?

Update depedencies to fix known security vulnerabiltiies #165

Uh oh!

Conversation

malor commented Jun 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

malor commented Jun 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!