Upgrade webapp dependencies #9916

timroes · 2022-01-31T11:02:10Z

What

Upgrade a bunch of used dependencies. It leaves a couple of major version updates of some dependencies unsolved, since they require more changes than I wanted to put into one PR.

How

See inline comments for more details where I actually changed code, and it wasn't simply a reformatting. I've also did a couple of minor updates where the code was anyway touched due to upgrade (also see inline comments).

I've tested that the pre commit hooks, building, running and running storybook are still working after this update.

timroes · 2022-01-31T11:02:42Z

airbyte-webapp/.eslintrc

@@ -33,5 +31,13 @@
      "warn"
    ]
  },
-  "parser": "@typescript-eslint/parser"
+  "parser": "@typescript-eslint/parser",
+  "overrides": [


ℹ️ Disable requiring import syntax in scripts, since they are running on node directly so should use require for now.

timroes · 2022-01-31T11:03:05Z

airbyte-webapp/.storybook/main.ts

@@ -1,4 +1,7 @@
 module.exports = {
+  core: {


ℹ️ Required for making storybook run with webpack 5, which is used now in the updated react-scripts version.

timroes · 2022-01-31T11:03:35Z

airbyte-webapp/scripts/license-check.js

@@ -56,27 +56,28 @@ const IGNORED_PACKAGES = [`airbyte-webapp@${version}`];
 * Overwrite licenses for specific packages manually, e.g. because they can't be detected properly.
 */
 const LICENSE_OVERWRITES = {
-  'glob-to-regexp@0.3.0': 'BSD-3-Clause',
-  'rework@1.0.1': 'MIT',


ℹ️ Removed license overwrite for rework since no longer needed.

timroes · 2022-01-31T11:11:23Z

airbyte-webapp/src/components/TextWithHTML/TextWithHTML.tsx

-      defaultMessage={sanitizedHtmlText}
-    />
-  );
+  return <span dangerouslySetInnerHTML={{ __html: sanitizedHtmlText }} />;


ℹ️ The FormattedHTMLMessage component doesn't exist anymore in newer versions of react-intl. Instead the FormattedMessage supports now all the same HTML. That said, in this place the text/HTML we print is comping directly from the specification, and there is no purpose putting it through react-intl, since it would still be a single id (textWithHtmlTags), such we can't translate it that way. If we'd want those property descriptions to be internationalized, we'd anyway need to build a system for that. Thus changing this to a regular dangerouslySetInnerHTML for now, which is safe in this place, because we used sanitize-html beforehand.

timroes · 2022-01-31T11:12:20Z

airbyte-webapp/src/components/TextWithHTML/TextWithHTML.tsx

+        target: "_blank",
+        rel: "noopener noreferrer",


ℹ️ Make sure all links are getting a proper rel tag to prevent the "opener attack" (see also https://mathiasbynens.github.io/rel-noopener/) in all browsers.

timroes · 2022-01-31T11:13:42Z

airbyte-webapp/src/config/configProviders.test.ts

@@ -29,7 +29,7 @@ describe("applyProviders", function () {
        innerProp: "1",
      },
    };
-    const providers: Provider<DeepPartial<Value>>[] = [
+    const providers: ProviderAsync<DeepPartial<Value>>[] = [


ℹ️ Fixed some broken type, that apparently slipped previous TS versions.

timroes · 2022-01-31T11:15:05Z

airbyte-webapp/src/packages/cloud/cloudRoutes.tsx

@@ -120,6 +120,7 @@ const MainViewRoutes = () => {
            path={`${r}/*`}
            element={
              <Navigate
+                // @ts-expect-error state is now unkown, needs proper typing


ℹ️ location.state is now unknown. I've properly typed it in one file, where I could encapsulate custom type guards only within this file. This place here might need some utility type guard somewhere else, so left it untyped for now.

timroes · 2022-01-31T11:15:46Z

airbyte-webapp/src/packages/cloud/views/auth/LoginPage/LoginPage.tsx

-            });
+          return (
+            login(values)
+              // @ts-expect-error state is now unkown, needs proper typing


ℹ️ Same untyped location.state as above.

timroes · 2022-01-31T11:18:03Z

airbyte-webapp/src/pages/ConnectionPage/pages/CreationFormPage/CreationFormPage.tsx

@@ -52,7 +71,7 @@ function usePreloadData(): {

  const source = useResource(
    SourceResource.detailShape(),
-    location.state?.sourceId
+    hasSourceId(location.state)


ℹ️ location.state is now unknown so added custom type guard in place to check for proper typing.

timroes · 2022-01-31T11:19:12Z

...-webapp/src/pages/DestinationPage/pages/CreateDestinationPage/components/DestinationForm.tsx

@@ -33,7 +44,9 @@ const DestinationForm: React.FC<IProps> = ({
  const analyticsService = useAnalyticsService();

  const [destinationDefinitionId, setDestinationDefinitionId] = useState(
-    location.state?.destinationDefinitionId || ""
+    hasDestinationDefinitionId(location.state)


ℹ️ Custom type guard for location.state, same as above.

timroes · 2022-01-31T11:19:37Z

airbyte-webapp/src/pages/SourcesPage/pages/CreateSourcePage/components/SourceForm.tsx

@@ -34,14 +45,13 @@ const SourceForm: React.FC<IProps> = ({
  const analyticsService = useAnalyticsService();

  const [sourceDefinitionId, setSourceDefinitionId] = useState(
-    location.state?.sourceDefinitionId || ""
+    hasSourceDefinitionId(location.state)


ℹ️ Custom type guard for location.state, same as above.

timroes · 2022-01-31T11:20:31Z

airbyte-webapp/src/views/Connector/ServiceForm/ServiceForm.test.tsx

@@ -304,6 +304,7 @@ describe("Service Form", () => {
      const submit = container.querySelector("button[type='submit']");
      await waitFor(() => userEvent.click(submit!));

+      // @ts-expect-error typed unknown, okay in test file


ℹ️ result.connectionConfiguration is explicitly typed as unknown and was like that already before the upgrade. I have no idea how the previous typescript version did not catch this 🤔 Anyway, since it's a test file, simply ignoring this for now.

timroes · 2022-01-31T11:20:42Z

airbyte-webapp/src/views/Connector/ServiceForm/ServiceForm.test.tsx

@@ -352,10 +354,6 @@ describe("Service Form", () => {
    });

    test("should fill right values oneOf field", async () => {
-      const credentials = screen.getByTestId(


ℹ️ Removed unused code.

timroes · 2022-01-31T11:21:37Z

airbyte-webapp/tsconfig.json

@@ -24,7 +24,8 @@
    "noImplicitReturns": true,
    "noImplicitThis": true,
    "noImplicitAny": true,
-    "noFallthroughCasesInSwitch": true
+    "noFallthroughCasesInSwitch": true,
+    "useUnknownInCatchVariables": false


ℹ️ By default TypeScript will now type the catch variable as unknown instead of any. This will though require a lot of more code changes whenever we catch exceptions. Thus disabling this for now, and continue typing the catch variable as any.

jamakase · 2022-01-31T11:49:21Z

First of all, we shouldn't update prettier config while we have other work in progress as it will lead to a lot of merge conflicts.

This work should be done only after connection form refactoring pr is merged as it has bunch of changes.

What is more, we already have a lot of work done of dependencies upgrade in another pr which also includes updates in libs with breaking changes ( storybook and etc ).

timroes · 2022-01-31T12:15:35Z

First of all, we shouldn't update prettier config while we have other work in progress as it will lead to a lot of merge conflicts.

We will going forward always have a lot of work in progress in parallel. I don't think we can hold of "all large PRs merged" for tasks like prettier update in the future. It's more important, whether it will collide timing wise on merging in the future. Since it seemed there is no other frontend PR currently close to being merged (today or tomorrow), I'd prefer doing it now while running into a situation where we also have other work nearly ready to be merged. Since all prettier changes are auto fixable by lint, we should just run a npm run lint -- --fix after this is merged on any open PR.

What is more, we already have a lot of work done of dependencies upgrade in another pr which also includes updates in libs with breaking changes ( storybook and etc ).

Could you add a link to this PR? I couldn't find any area/frontend PR that did some major upgrades to dependencies?

timroes · 2022-01-31T12:26:41Z

This work should be done only after connection form refactoring pr is merged as it has bunch of changes.

What is the timeframe, on when this is planned to be merged?

jamakase · 2022-01-31T12:27:01Z

Yes, but merge conflicts can't be addressed automatically with prettier. It means if prettier is changed, all other prs should be updated then. And they will have bunch of merge conflicts as prettier will be changed in master.

As prettier is just a tool and it is not a hard requirement in development process - updating it shouldn't lead to inconveniences for other team members. If it leads - it means that its update should be postponed for better timing.

What is the reason behind updating default prettier styling without any prior notice for other team members? I do not see why it couldn't wait a bit.

If you could take care of merging these prettier changes in all other branches and solving merge conflicts - then I have nothing against updating it now.

As for fixes - there is a branch in named jamakase/fix-webapp-dependencies

timroes · 2022-01-31T12:55:49Z

Removed the prettier upgrade from this PR, also postponing for discussion to our next weekly.

timroes · 2022-02-21T16:41:41Z

Superseeded by #10507

Tim Roes added 6 commits January 28, 2022 19:22

Upgrade minor dependencies

62862f8

WIP react-intl update

2389c01

Merge branch 'master' into webapp/upgrade-dependencies

3601beb

Improve eslint license-check

0571ed5

Apply rel tag automatically

0731b71

Upgrade storybook and more

93fbc44

timroes added the area/frontend label Jan 31, 2022

timroes requested review from jrhizor and jamakase January 31, 2022 11:02

github-actions bot added the area/platform issues related to the platform label Jan 31, 2022

timroes commented Jan 31, 2022

View reviewed changes

Remove unused code

0d808e3

timroes commented Jan 31, 2022

View reviewed changes

Remove unused eslint ignore

5ab9c0e

timroes mentioned this pull request Jan 31, 2022

Tim/webapp/upgrade dependencies #9919

Closed

timroes temporarily deployed to more-secrets January 31, 2022 11:29 Inactive

This was referenced Jan 31, 2022

Bump ws from 6.2.1 to 6.2.2 in /airbyte-webapp #9879

Merged

Bump nanoid from 3.1.23 to 3.2.0 in /airbyte-webapp #9878

Closed

timroes mentioned this pull request Jan 31, 2022

Bump ssri from 6.0.1 to 6.0.2 in /airbyte-webapp #9877

Closed

Remove prettier upgrade

0855e19

timroes temporarily deployed to more-secrets January 31, 2022 12:54 Inactive

timroes marked this pull request as draft January 31, 2022 12:54

timroes removed the request for review from jrhizor January 31, 2022 12:55

Merge branch 'master' into tim/webapp/upgrade-dependencies

0cf989b

timroes closed this Feb 21, 2022

timroes temporarily deployed to more-secrets February 21, 2022 16:42 Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade webapp dependencies #9916

Upgrade webapp dependencies #9916

timroes commented Jan 31, 2022 •

edited

Loading

timroes Jan 31, 2022

timroes Jan 31, 2022

timroes Jan 31, 2022

timroes Jan 31, 2022

timroes Jan 31, 2022

timroes Jan 31, 2022

timroes Jan 31, 2022

timroes Jan 31, 2022

timroes Jan 31, 2022

timroes Jan 31, 2022

timroes Jan 31, 2022

timroes Jan 31, 2022

timroes Jan 31, 2022

timroes Jan 31, 2022

jamakase commented Jan 31, 2022

timroes commented Jan 31, 2022 •

edited

Loading

timroes commented Jan 31, 2022

jamakase commented Jan 31, 2022

timroes commented Jan 31, 2022

timroes commented Feb 21, 2022

Upgrade webapp dependencies #9916

Upgrade webapp dependencies #9916

Conversation

timroes commented Jan 31, 2022 • edited Loading

What

How

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

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

jamakase commented Jan 31, 2022

timroes commented Jan 31, 2022 • edited Loading

timroes commented Jan 31, 2022

jamakase commented Jan 31, 2022

timroes commented Jan 31, 2022

timroes commented Feb 21, 2022

timroes commented Jan 31, 2022 •

edited

Loading

timroes commented Jan 31, 2022 •

edited

Loading