feat(ui): bump frontend dependencies #8353

ngamanda · 2023-06-30T15:16:34Z

There's a whole bunch of lint warnings now. I'm setting them to warn for your review. Let me know if you'd prefer to turn them off or just bulk update them.

Bump dependencies
- Cleaned up duplicated packages in yarn.lock
- Migrated @vs packages to @visx
- Remove implementation for @data-ui/xy-chart to @visx/xychart
Fix ESLint config, VSCode now shows all lint errors
- Removed unused plugins
- Updated AirBnB plugins
- Disabled/warn the newly added/changed rules
Bump Gradle node version to LTS
Remove 'react-email-share-link'
- Had some trouble compiling it, so I opted to directly copy the implementation over instead
Configure jest to use moduleNameMapper to map packages rather than transpiling them again

Checklist

The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
Links to related issues (if applicable)
Tests for the changes have been added/updated (if applicable)
Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

joshuaeilers · 2023-09-07T22:49:13Z

Hey @ngamanda - so sorry for the delay on this review. I'm pulling down the code now and going to do a little test drive with it and then start reviewing shortly after. There's a coupe conflicts, but I'll do my local testing rebased on master.

joshuaeilers · 2023-09-08T00:02:58Z

datahub-web-react/.eslintrc.js

-        'no-console': 'off',
-        'no-debugger': 'warn',
-        'require-await': 'warn',
+        '@typescript-eslint/no-explicit-any': 'warn',


I think my preference here would be to try to make this upgrade as transparent as possible, for now. While it's more ideal to fix a lot of these, doing this in stages where we upgrade with the warnings=off, and then can address one at a time. Helps keep the changeset a bit smaller. Also keeps the console build warnings smaller. Again, I know it's sweeping under the rug in the short term, but I'd rather not have to make every yarn start show a ton of warnings.

Sure, I'll swap all the warnings to off then. You mentioned you tried rebasing locally - will you be updating this MR with the resolved conflicts or should I tackle that instead?

I guess more specifically if you could set the warnings to match what the eslintrc had before, that would be great 👍 Not sure if they all really need to be necessarily off if we had some to warn before, thinking to keep parity unless there's a reason we can't do that.

I won't be able to directly edit this PR due to permissions, so if you can do that here that would be ideal.

The good news is CI passed on my test branch, so I'm gonna plan to do a bit more testing on it today. Then once that's feeling good, we can figure out a time to merge. One thing about this kind of PR is that it can lead to some interruption on developers side so we may want to do a little announcement or something that folks need to re-yarn / upgrade node ideally post-merging this.

But we can handle that post-merge of course

joshuaeilers · 2023-09-08T00:04:24Z

datahub-web-react/build.gradle

@@ -16,7 +16,7 @@ node {
  }

  // Version of node to use.
-  version = '16.8.0'
+  version = '16.20.1'


Thank you so much for this!

joshuaeilers · 2023-09-08T00:08:23Z

datahub-web-react/craco.config.js

@@ -1,8 +1,11 @@
+/* eslint-disable @typescript-eslint/no-var-requires */


Nice, this kinda stuff for webpack upgrades is really tricky to figure out 🙏

datahub-web-react/src/app/shared/share/items/EmailMenuItem.tsx

joshuaeilers · 2023-09-11T16:39:13Z

@ngamanda I'm noticing the analytics tab's charts x axes values look wrong they appear to be numeric, and I'm guessing it's trying to format a timestamp as a number instead of a date. Could you take a look at that?

joshuaeilers · 2023-09-11T16:40:03Z

Here's a screenshot of what I'm seeing, analytics at the top-right.

joshuaeilers

Setting to changes requested due to the x-axis issue on the charts.

ngamanda · 2023-09-13T16:00:48Z

Hey @joshuaeilers, thanks for catching that! Just committed a fix - lmk if there's any other issues!

joshuaeilers · 2023-09-13T19:05:53Z

This is looking good to me. Tagging in @chriscollins3456 in for the final review and discuss merge timelines. I know we have a couple fairly critical frontend changes coming in this month so we may want to schedule this in October for stability sake, but I'll defer to Chris on that. Thank you again so much for this work!

chriscollins3456

this is looking great! I pulled it down and built everything from scratch and it's looking good to me. the changes make sense and I think this is going to resolve a good amount of our vulnerability issues with older versions of packages as well. thanks so much for this @ngamanda !

I'm going to double check with the team before merging but I think we're close to good to go here

chriscollins3456 · 2023-11-21T16:35:09Z

datahub-web-react/craco.config.js

+        configure: {
+            optimization: whenProd(() => ({
+                splitChunks: {
+                    cacheGroups: {
+                        vendor: {
+                            test: /[\\/]node_modules[\\/]/,
+                            name: 'vendors',
+                            chunks: 'all',
+                        },
+                    },
+                },
+            })),
+            // Webpack 5 no longer automatically pollyfill core Node.js modules
+            resolve: { fallback: { fs: false } },
+            // Ignore Webpack 5's missing source map warnings from node_modules
+            ignoreWarnings: [{ module: /node_modules/, message: /source-map-loader/ }],
+        },


nice I'm excited about this! hopefully reduces our build size a bit

chriscollins3456 · 2023-11-21T16:38:17Z

datahub-web-react/.eslintrc.js

+        'no-unsafe-optional-chaining': 'off',
+        'prefer-exponentiation-operator': 'off',
+        'prefer-regex-literals': 'off',
+        'react/destructuring-assignment': 'off',
+        'react/function-component-definition': 'off',
+        'react/jsx-no-bind': 'off',
+        'react/jsx-no-constructed-context-values': 'off',
+        'react/jsx-no-useless-fragment': 'off',
+        'react/jsx-props-no-spreading': 'off',
+        'react/no-unstable-nested-components': 'off',
+        'react/require-default-props': 'off',


so these lint changes were necessary as we bumped dependencies we had new linting rules being applied that we didn't want applied?

nothing I'm seeing is concerning to me just wanted to call it out

Yeap! That's right 😃

chriscollins3456 · 2023-11-27T16:45:10Z

@ngamanda thank you so much again! just merged in latest and then pulled and ran locally again and we're looking good to go. going to merge now!

github-actions bot added the product PR or Issue related to the DataHub UI/UX label Jun 30, 2023

vercel bot deployed to Preview June 30, 2023 15:30 View deployment

anshbansal added the community-contribution PR or Issue raised by member(s) of DataHub Community label Jul 17, 2023

ngamanda force-pushed the bump-frontend-dependencies branch from 00898c6 to 70761c0 Compare July 24, 2023 03:37

vercel bot deployed to Preview July 24, 2023 03:50 View deployment

maggiehays assigned joshuaeilers Aug 9, 2023

joshuaeilers mentioned this pull request Sep 7, 2023

(DO NOT MERGE) CI Testing for: Bump frontend dependencies #8806

Closed

joshuaeilers reviewed Sep 8, 2023

View reviewed changes

datahub-web-react/src/app/shared/share/items/EmailMenuItem.tsx Show resolved Hide resolved

ngamanda force-pushed the bump-frontend-dependencies branch from 70761c0 to 0ee1e09 Compare September 9, 2023 15:04

vercel bot deployed to Preview September 9, 2023 15:20 View deployment

joshuaeilers suggested changes Sep 11, 2023

View reviewed changes

ngamanda added 12 commits September 14, 2023 00:03

de

81b15f0

update dependencies

d602180

remove react-email-share-link since it's not compiled

6ce9120

disable check for MarkdownViewer

18ba02b

supress source map warnings for webpack 5

2d58bd4

update vs to visx

585026b

remove urn writeText to avoid 'Document not focused error'

32bf172

Refactor timeseries chart to use @visx

a97c274

Fix jest test module mappings

4ca0d11

Add comments

c2ff029

update target to es2017

f05ff28

bump node

7fbdd13

ngamanda added 9 commits September 14, 2023 00:03

update lint errors

c77e35b

bump d3-scale to match @visx

6c67882

bump copy-webpack-plugin

7aedd5c

bump node

51eabf5

fix lint

d56df0e

remove sacle config

567ab19

fix import

6b816eb

update rules

c596644

fix timeseries chart

31a6532

ngamanda force-pushed the bump-frontend-dependencies branch from 30686b1 to 31a6532 Compare September 13, 2023 16:09

vercel bot deployed to Preview September 13, 2023 16:49 View deployment

joshuaeilers requested a review from chriscollins3456 September 13, 2023 19:05

joshuaeilers approved these changes Sep 13, 2023

View reviewed changes

maggiehays assigned chriscollins3456 and unassigned joshuaeilers Nov 13, 2023

Merge branch 'master' into bump-frontend-dependencies

cb65275

chriscollins3456 approved these changes Nov 21, 2023

View reviewed changes

vercel bot deployed to Preview November 21, 2023 16:43 View deployment

add back dependency in yarn lock removed from conflict resolution

87a7b79

vercel bot deployed to Preview November 21, 2023 18:42 View deployment

Merge branch 'master' into bump-frontend-dependencies

ac39112

vercel bot deployed to Preview November 27, 2023 15:33 View deployment

chriscollins3456 merged commit a9650b6 into datahub-project:master Nov 27, 2023
35 of 36 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ui): bump frontend dependencies #8353

feat(ui): bump frontend dependencies #8353

ngamanda commented Jun 30, 2023 •

edited

Loading

joshuaeilers commented Sep 7, 2023

joshuaeilers Sep 8, 2023 •

edited

Loading

ngamanda Sep 8, 2023

joshuaeilers Sep 8, 2023

joshuaeilers Sep 8, 2023

joshuaeilers Sep 8, 2023

joshuaeilers Sep 8, 2023

joshuaeilers Sep 8, 2023

joshuaeilers Sep 8, 2023

joshuaeilers commented Sep 11, 2023

joshuaeilers commented Sep 11, 2023

joshuaeilers left a comment

ngamanda commented Sep 13, 2023

joshuaeilers commented Sep 13, 2023

chriscollins3456 left a comment

chriscollins3456 Nov 21, 2023

chriscollins3456 Nov 21, 2023

ngamanda Nov 22, 2023

chriscollins3456 commented Nov 27, 2023

		@@ -1,8 +1,11 @@
		/* eslint-disable @typescript-eslint/no-var-requires */

feat(ui): bump frontend dependencies #8353

feat(ui): bump frontend dependencies #8353

Conversation

ngamanda commented Jun 30, 2023 • edited Loading

Checklist

joshuaeilers commented Sep 7, 2023

joshuaeilers Sep 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshuaeilers commented Sep 11, 2023

joshuaeilers commented Sep 11, 2023

joshuaeilers left a comment

Choose a reason for hiding this comment

ngamanda commented Sep 13, 2023

joshuaeilers commented Sep 13, 2023

chriscollins3456 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chriscollins3456 commented Nov 27, 2023

ngamanda commented Jun 30, 2023 •

edited

Loading

joshuaeilers Sep 8, 2023 •

edited

Loading