Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Audit unused packages #595

Open
BSFishy opened this issue Mar 13, 2023 · 29 comments
Open

Audit unused packages #595

BSFishy opened this issue Mar 13, 2023 · 29 comments
Assignees
Labels
good first issue Good for newcomers help wanted Extra attention is needed refactor Research An open request for eng to research one or more topics

Comments

@BSFishy
Copy link
Contributor

BSFishy commented Mar 13, 2023

Now would be a good time to go through the list of packages in OUI and the list of resolutions and figure out if they're still necessary. It's possible that over time, packages have stopped being used or resolutions are no longer necessary.

It might be better to take care of this task after we complete #594

@BSFishy BSFishy added Research An open request for eng to research one or more topics refactor labels Mar 13, 2023
@BSFishy BSFishy removed the untriaged label Mar 13, 2023
@abbyhu2000 abbyhu2000 added good first issue Good for newcomers help wanted Extra attention is needed CCI College Contributor Initiative and removed CCI College Contributor Initiative labels Apr 11, 2023
@BSFishy
Copy link
Contributor Author

BSFishy commented May 18, 2023

We can start with this:

Extra dependencies (15) - dependencies specified in the "dependencies" section but aren't used in any application code: [
  "@types/chroma-js",
  "@types/lodash",
  "@types/numeral",
  "@types/react-beautiful-dnd",
  "@types/react-input-autosize",
  "@types/react-virtualized-auto-sizer",
  "@types/react-window",
  "@types/refractor",
  "@types/resize-observer-browser",
  "@types/vfile-message",
  "numeral",
  "rehype-raw",
  "rehype-stringify",
  "url-parse",
  "uuid"
]
Unspecified dependencies (21) - dependencies used in application code but aren't specified in any section: [
  "@babel/template",
  "@babel/types",
  "@elastic/charts",
  "@faker-js/faker",
  "@opensearch-project/oui",
  "codesandbox",
  "enzyme",
  "history",
  "html",
  "html-format",
  "pegjs-inline-precompile",
  "react-helmet",
  "react-redux",
  "react-router",
  "react-router-dom",
  "react-router-redux",
  "react-view",
  "redux",
  "redux-thunk",
  "unist",
  "vfile-message"
]

(from https://gist.github.com/BSFishy/08d48577849391eb08c9313ae632574b)

@BigSamu
Copy link
Contributor

BigSamu commented Oct 9, 2023

@BSFishy

I am interested in taking this issue. I am wondering if there is any tool that can help analyze the whole code and check the % of the usage of packages. Issue #594 as been sorted out to start working on this?

@BSFishy
Copy link
Contributor Author

BSFishy commented Oct 9, 2023

I am wondering if there is any tool that can help analyze the whole code and check the % of the usage of packages.

I am not aware of any tools that perform this check. You can see in #595 (comment) I attempted to make a little script to perform this, but that script most definitely has some issues and may be better used as a starting point rather than a de-facto source of truth.

Issue #594 as been sorted out to start working on this?

Issue #594 is still in progress, and may be for a good amount of time. We're working on it incrementally, because a full upgrade of every package in the list of dependencies would involve a lot of moving parts (just take a look at #952, #949, #770, etc), so it's easier to tackle for all parties involved if it's taken incrementally.

I think this can be done incrementally in tandem. We can do an initial pass now, and as we improve our dependencies, we can continue work on working out resolutions and unused packages.

@BigSamu
Copy link
Contributor

BigSamu commented Oct 10, 2023

@BSFishy, Got it. Let's start the review incrementally. Although it is going to take time, I think. First step, I will check all the listed dependencies you mention in comment #595 (comment) and check if they are being used or not. What do you think?

@BSFishy
Copy link
Contributor Author

BSFishy commented Oct 10, 2023

First step, I will check all the listed dependencies you mention in comment #595 (comment) and check if they are being used or not. What do you think?

Yeah, I think that's a great starting place

@BigSamu
Copy link
Contributor

BigSamu commented Oct 15, 2023

@BSFishy,

Just finished an entire review ONLY for the dependencies (not devDependencies). Unfortunetely, the list you gave before is not entirely accurate, so what I did was to remove one by one each dependency with a test after the removal.

Below a lists of the dependencies that upon removal, the tests still pass and the oui website documentation load with no apparent problem.

"chroma-js": "^2.4.2",
"lodash": "^4.17.21",
"@types/resize-observer-browser": "^0.1.7",
"vfile": "^4.2.0",
"mdast-util-to-hast": "^10.0.0",
"prop-types": "^15.8.1",
"react-is": "~16.3.0",
"rehype-raw": "^5.0.0",
"rehype-stringify": "^8.0.0",
"unist-util-visit": "^2.0.3",
"url-parse": "^1.5.10",
"uuid": "^9.0.0"

Here the entire command to remove all these packages if you want to test on your side

yarn remove chroma-js lodash @types/resize-observer-browser vfile mdast-util-to-hast prop-types react-is rehype-raw rehype-stringify unist-util-visit url-parse uuid

Two questions regarding these results:

  • For the list of dependencies I removed in which all tests were passed, can we be sure that those packages can be removed with no future problems in the base code? I am not sure if we need to check something else.
  • Would it be possible to check the script you created to make easier the review of the devDependencies? I think the examination for those ones would be more tricky and I am not sure if we can rely only on the suite tests to safely remove a package of this type. Any suggestions on how to proceed here?

Regards,

Samuel

@BSFishy
Copy link
Contributor Author

BSFishy commented Oct 16, 2023

Thanks for doing this so far. I've written a new (smarter but still dumb) script script, which reads whatever is currently in package.json's dependencies, devDependencies, and peerDependencies then scours src, src-docs, and scripts and just does a dummy "does this file contents contain this dep". The source code can be found here. The output that I got from that script on main is

Dependencies which aren't used directly anywhere in the code - [
  "@babel/cli",
  "@babel/plugin-proposal-class-properties",
  "@babel/plugin-proposal-object-rest-spread",
  "@babel/plugin-transform-async-to-generator",
  "@babel/plugin-transform-runtime",
  "@babel/preset-env",
  "@babel/preset-react",
  "@elastic/eslint-config-kibana",
  "@typescript-eslint/eslint-plugin",
  "@typescript-eslint/parser",
  "autoprefixer",
  "babel-core",
  "babel-plugin-add-module-exports",
  "babel-plugin-dynamic-import-node",
  "babel-plugin-inline-react-svg",
  "babel-plugin-pegjs-inline-precompile",
  "core-js",
  "cross-env",
  "eslint-config-prettier",
  "eslint-import-resolver-webpack",
  "eslint-plugin-babel",
  "eslint-plugin-import",
  "eslint-plugin-jest",
  "eslint-plugin-jsx-a11y",
  "eslint-plugin-local",
  "eslint-plugin-mocha",
  "eslint-plugin-prefer-object-spread",
  "eslint-plugin-prettier",
  "eslint-plugin-react",
  "eslint-plugin-react-hooks",
  "jest-cli",
  "moment-timezone",
  "postcss-cli",
  "postcss-inline-svg",
  "pre-commit",
  "rehype-raw",
  "rehype-stringify",
  "sass-lint-auto-fix",
  "start-server-and-test",
  "webpack-cli",
  "webpack-dev-server",
  "yeoman-generator"
]

HOWEVER, I know some of those dependencies are used elsewhere, outside of those directories. For example, we use cross-env in our scripts to set environment variables on any platform. So again, take this list with a grain of salt, as there are most definitely some dependencies listed, yet are absolutely used. Likewise, the way that I check if a dependency is used in a file is a fairly naive method, so it's possible that there are some false-negatives too.

The reason I did this is because I can see some of the dependencies you listed are most definitely used. For example, chroma-js is used directly in application code:

import chroma from 'chroma-js';

import chroma, { ColorSpaces } from 'chroma-js';

I think ultimately, the best way to figure out if a dependency is used is to manually validate. Meaning to do a full project search for the dependency name and manually validating false positives and negatives.

  • For the list of dependencies I removed in which all tests were passed, can we be sure that those packages can be removed with no future problems in the base code? I am not sure if we need to check something else.

  • Would it be possible to check the script you created to make easier the review of the devDependencies? I think the examination for those ones would be more tricky and I am not sure if we can rely only on the suite tests to safely remove a package of this type. Any suggestions on how to proceed here?

  • I think the best way of knowing will be to manually validate as I mentioned above. One other thing we can do is generate a build and run OSD's test suite against it. OSD has a lot of actual interaction testing and the sort, so those tests should catch a lot more of the issues which may arise from these sorts of changes. I don't think we have any documentation on that, so I can make a new issue to write some.
  • Yeah, I wrote a new script (source above), which should be a little more definitive, but again, I think manual validation would be the best way to guarantee we're not breaking anything. On a tangent, there are a few scripts I usually like to run to ensure our build process (dev dependencies) are working correctly:
    • yarn start
    • yarn build
    • yarn build-docs
    • yarn test
    • yarn lint

I'll also say that understanding each individual dependency and how it may interact with our setup helps a lot. For example, a lot of tools don't come with a CLI built in, so you need to install a CLI package for that. E.g. jest-cli

Hope this helps

@BSFishy
Copy link
Contributor Author

BSFishy commented Oct 16, 2023

As part of this, I'd also like to see dependencies get put in their correct sections. For example, it doesn't make much sense to me that @types/ dependencies would be in the dependencies section, and would make more sense being in the devDependencies section. I think, again, this will probably be a lot of manual validation.

@JohnathonBowers
Copy link
Contributor

Hi, @BSFishy . @BigSamu and I talked yesterday about this issue, and we were wondering if I could be added to the issue so that he and I can work on it together? What would be the best way for us to work on the same branch, since we have our own separate forked repos? My thought is that he could give me contributor access to the branch he is doing the audit on. However, I wanted to run that idea by you and see if there is a better way to do things.

@BSFishy
Copy link
Contributor Author

BSFishy commented Oct 17, 2023

Hi, @BSFishy . @BigSamu and I talked yesterday about this issue, and we were wondering if I could be added to the issue so that he and I can work on it together? What would be the best way for us to work on the same branch, since we have our own separate forked repos? My thought is that he could give me contributor access to the branch he is doing the audit on. However, I wanted to run that idea by you and see if there is a better way to do things.

Yeah, that sounds great. It really depends on how close you're working on things. If the surface area you're covering is relatively small, contributor permissions would probably be the way to go. The other methods that come to mind are making another fork off of @BigSamu's fork and making PRs against that one, then make one final PR to this repo, or making a feature branch in this repo then both of you can make PRs into that branch, but that's usually reserved for larger, long-running projects. Given this will most likely mainly be changes to package.json and yarn.lock, I think contributor access will be the way to go :)

@BigSamu
Copy link
Contributor

BigSamu commented Oct 17, 2023

Hi, @BSFishy . @BigSamu and I talked yesterday about this issue, and we were wondering if I could be added to the issue so that he and I can work on it together? What would be the best way for us to work on the same branch, since we have our own separate forked repos? My thought is that he could give me contributor access to the branch he is doing the audit on. However, I wanted to run that idea by you and see if there is a better way to do things.

Yeah, that sounds great. It really depends on how close you're working on things. If the surface area you're covering is relatively small, contributor permissions would probably be the way to go. The other methods that come to mind are making another fork off of @BigSamu's fork and making PRs against that one, then make one final PR to this repo, or making a feature branch in this repo then both of you can make PRs into that branch, but that's usually reserved for larger, long-running projects. Given this will most likely mainly be changes to package.json and yarn.lock, I think contributor access will be the way to go :)

Hi @BSFishy, yeah I think the contributor approach will be the way to go. The review of the packages is big for this issue. So some extra hands would be really appreciated. Just wonder if when we finished this review and send a PRs, this PRs will be co-author by both of us, correct?

@BSFishy
Copy link
Contributor Author

BSFishy commented Oct 17, 2023

Hi @BSFishy, yeah I think the contributor approach will be the way to go. The review of the packages is big for this issue. So some extra hands would be really appreciated. Just wonder if when we finished this review and send a PRs, this PRs will be co-author by both of us, correct?

Only one person will make the PR, however the commits within the PR will have their respective authors. You can look into how to add multiple authors to a commit here: https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors (make sure you're also signing the commits off too!)

From there, when the PR is merged, it will show up as being authored by both of you

@BigSamu
Copy link
Contributor

BigSamu commented Oct 17, 2023

Hi @BSFishy, yeah I think the contributor approach will be the way to go. The review of the packages is big for this issue. So some extra hands would be really appreciated. Just wonder if when we finished this review and send a PRs, this PRs will be co-author by both of us, correct?

Only one person will make the PR, however the commits within the PR will have their respective authors. You can look into how to add multiple authors to a commit here: https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors (make sure you're also signing the commits off too!)

From there, when the PR is merged, it will show up as being authored by both of you

Great! Thanks! We will take a look to that!

@BigSamu
Copy link
Contributor

BigSamu commented Oct 23, 2023

@JohnathonBowers,

I am starting the review of the devDependecies this week. Because the list is huge, would you like to start from the bottom and I start from the top? I think this is the best approach. Also, we should check if the list of dependencies saved there are indeed devDependencies. It could be some of them wrongly classified.

Regards,

Samuel

@BigSamu
Copy link
Contributor

BigSamu commented Oct 23, 2023

@BSFishy,

Continue with the reviewing of the runtime dependencies and taking into consideration your comment above the list below enumerate the packages that are not being used

"@types/resize-observer-browser": "^0.1.7",
"rehype-raw": "^5.0.0",
"rehype-stringify": "^8.0.0"

Also, in the same original the following packages should change their classification from dependencies to devDependencies:

"@types/chroma-js": "^2.4.0",
"@types/lodash": "4.14.192",
"@types/numeral": "^2.0.2",
"@types/react-beautiful-dnd": "^13.1.3",
"@types/react-input-autosize": "^2.2.1",
"@types/react-virtualized-auto-sizer": "^1.0.1",
"@types/react-window": "^1.8.5",
"@types/refractor": "^3.0.0",
"@types/resize-observer-browser": "^0.1.7",

Here the proper commands for this updates:

yarn remove @types/resize-observer-browser rehype-raw rehype-stringify
yarn remove @types/chroma-js @types/lodash @types/numeral @types/react-beautiful-dnd @types/react-input-autosize @types/react-virtualized-auto-sizer @types/react-window @types/refractor 
yarn add --dev @types/chroma-js @types/lodash @types/numeral @types/react-beautiful-dnd @types/react-input-autosize @types/react-virtualized-auto-sizer @types/react-window @types/refractor 

Sending this message to update @JohnathonBowers. @JohnathonBowers I will start the review with this list shared by @BSFishy from top to bottom. Then later, we can check the entire list

Dependencies which aren't used directly anywhere in the code - [
  "@babel/cli",
  "@babel/plugin-proposal-class-properties",
  "@babel/plugin-proposal-object-rest-spread",
  "@babel/plugin-transform-async-to-generator",
  "@babel/plugin-transform-runtime",
  "@babel/preset-env",
  "@babel/preset-react",
  "@elastic/eslint-config-kibana",
  "@typescript-eslint/eslint-plugin",
  "@typescript-eslint/parser",
  "autoprefixer",
  "babel-core",
  "babel-plugin-add-module-exports",
  "babel-plugin-dynamic-import-node",
  "babel-plugin-inline-react-svg",
  "babel-plugin-pegjs-inline-precompile",
  "core-js",
  "cross-env",
  "eslint-config-prettier",
  "eslint-import-resolver-webpack",
  "eslint-plugin-babel",
  "eslint-plugin-import",
  "eslint-plugin-jest",
  "eslint-plugin-jsx-a11y",
  "eslint-plugin-local",
  "eslint-plugin-mocha",
  "eslint-plugin-prefer-object-spread",
  "eslint-plugin-prettier",
  "eslint-plugin-react",
  "eslint-plugin-react-hooks",
  "jest-cli",
  "moment-timezone",
  "postcss-cli",
  "postcss-inline-svg",
  "pre-commit",
  "rehype-raw",
  "rehype-stringify",
  "sass-lint-auto-fix",
  "start-server-and-test",
  "webpack-cli",
  "webpack-dev-server",
  "yeoman-generator"
]

@JohnathonBowers
Copy link
Contributor

@BigSamu That sounds good. I can start from the bottom of the list and work up.

@BigSamu
Copy link
Contributor

BigSamu commented Oct 24, 2023

@JohnathonBowers,

As @BSFishy mentioned in his post above, remember to use the following scripts for checking the list of devDependencies he shared with us

yarn start
yarn build
yarn build-docs
yarn test
yarn lint

@BigSamu
Copy link
Contributor

BigSamu commented Oct 26, 2023

Ok, some progress has been made in this issue. Here a google docs we are using for the review of the dev dependencies according to the list given by @BSFishy above. Review for runtime dependencies already already finished.

@JohnathonBowers
Copy link
Contributor

@BSFishy I'm working up from the bottom of the list you gave to @BigSamu . The first dependency I found that was unused in the source code is sass-lint-auto-fix. I uninstalled the dependency and ran yarn start, yarn build, yarn build-docs, and yarn test. Everything ran without issue. However, I'm not sure if we should remove the dependency, since it is a complement to sass-lint and gives developers the ability to resolve linting errors through the CLI. I'm guessing that we should leave the dependency installed?

@BigSamu
Copy link
Contributor

BigSamu commented Oct 27, 2023

Ok @Willie-The-Lord is joining efforts here to continue working on this issue. From my side, I finished a review in the dev dependencies from package @babel/cli to Cross-env (check google docs). @JohnathonBowers is doing the check on the list starting from the bottom. @Willie-The-Lord can you continue the review from where I left, starting with the eslint-config-prettier and going through all eslint packages? Important, leave a comment in the Google Docs about whether the package is being used or not.

NOTE: One milestone has been reached with the review of the runtime dependencies and reclassification of dev dependencies wrong classified under runtime dependencies. Please check this last commit in the working repo.

@BSFishy two questions for you:

Regards,

Samuel

@BSFishy
Copy link
Contributor Author

BSFishy commented Oct 27, 2023

@BSFishy I'm working up from the bottom of the list you gave to @BigSamu . The first dependency I found that was unused in the source code is sass-lint-auto-fix. I uninstalled the dependency and ran yarn start, yarn build, yarn build-docs, and yarn test. Everything ran without issue. However, I'm not sure if we should remove the dependency, since it is a complement to sass-lint and gives developers the ability to resolve linting errors through the CLI. I'm guessing that we should leave the dependency installed?

Yeah, checking the package.json, the lint-sass-fix script uses this package, so it wouldn't be safe to remove at this point.

Let's create a new issue for this and discuss it there.

  • Have you heard about the tool deepcheck? I heard about to when working with ChatGPT. Do you think it will be a useful tool for working along this issue?

This tool looks like a smarter version of the script I wrote. I think ideally, a tool will actually parse and understand how code is included in the project, which it looks like depcheck does. Of course, a final manual review just to be 100% sure it's accurate in the context of this project would be ideal, since every project is different and for that reason, these tools can't be 100% accurate 100% of the time. But yeah, for sure see what that tool says about OUI!

@BigSamu
Copy link
Contributor

BigSamu commented Oct 31, 2023

Let's create a new issue for this and discuss it there.

@BSFishy, Issue #1130 created for this.

This tool looks like a smarter version of the script I wrote. I think ideally, a tool will actually parse and understand how code is included in the project, which it looks like depcheck does. Of course, a final manual review just to be 100% sure it's accurate in the context of this project would be ideal, since every project is different and for that reason, these tools can't be 100% accurate 100% of the time. But yeah, for sure see what that tool says about OUI!

Ok I will use it for a final check!

@JohnathonBowers
Copy link
Contributor

@Willie-The-Lord I've checked the dependencies from the bottom of Matt's list up until the es-lint dependencies. Are you able to check the es-lint dependencies, or would you like me to do that?

@willie-hung
Copy link
Contributor

Hi @JohnathonBowers I will be checking the dependencies later today!

@willie-hung
Copy link
Contributor

@BigSamu Just audited the unused packages and push the changes to your branch. I also updated the google doc. Thanks!

@BigSamu
Copy link
Contributor

BigSamu commented Nov 1, 2023

@BigSamu Just audited the unused packages and push the changes to your branch. I also updated the google doc. Thanks!

Many thanks @Willie-The-Lord!

@BSFishy, a resume so far of the work

  1. We have removed the following dependencies:
"@types/resize-observer-browser": "^0.1.7",
"rehype-raw": "^5.0.0",
"rehype-stringify": "^8.0.0"
"moment-timezone": "^0.5.41",
"eslint-plugin-jest": "^24.1.0",
  1. The following dependencies below were found that are deprecated and issue [OSCI] Plugin Babel Dependencies Deprecated (Proposal-Class-Properties and Proposal-Object-Rest-Spread) #1130 was created to address this.
"@babel/plugin-proposal-class-properties": "^7.10.4",
"@babel/plugin-proposal-object-rest-spread": "^7.11.0",
  1. the dependencies below, were reclassified as devDependencies instead of runtime dependencies
"@types/chroma-js": "^2.4.0",
"@types/lodash": "4.14.192",
"@types/numeral": "^2.0.2",
"@types/react-beautiful-dnd": "^13.1.3",
"@types/react-input-autosize": "^2.2.1",
"@types/react-virtualized-auto-sizer": "^1.0.1",
"@types/react-window": "^1.8.5",
"@types/refractor": "^3.0.0",
  1. A review was than using the tool deepcheck, getting the outputs below. For those result I recommend to perform further investigation after finishing issue Audit outdated dependencies #594, because the output are not quite correct
➜  oui git:(clean/unused-packages) ✗ depcheck                                                                              [🐍 system]
Unused devDependencies
* @babel/cli
* @babel/plugin-transform-async-to-generator
* @babel/plugin-transform-runtime
* @svgr/plugin-jsx
* @svgr/plugin-svgo
* @types/jest
* babel-core
* babel-jest
* cache-loader
* core-js
* enzyme-to-json
* eslint-import-resolver-webpack
* postcss-cli
* pre-commit
* webpack-cli
Missing dependencies
* history: ./src-docs/src/routes.js
* variables-from-scss: ./src-docs/src/views/utility_classes/utility_classes_responsive.js
* prop-loader: ./src-docs/src/views/tables/sorting/sorting_section.js
* @babel/types: ./src-docs/src/views/panel/playground.js
* brace: ./src-docs/src/views/code_editor/code_editor.js
* @babel/template: ./src-docs/src/services/playground/utils.js
* @opensearch-project/oui: ./src/components/index.d.ts
* pegjs-inline-precompile: ./src/components/search_bar/query/default_syntax.ts
* portal: ./src/components/portal/__snapshots__/_index.scss
* vfile-message: ./src/components/markdown_editor/markdown_editor.tsx
* unist: ./src/components/markdown_editor/markdown_types.ts
* cheerio: ./src/components/icon/icon.test.tsx
* postcss: ./scripts/compile-themes.js
* <%= fileName %>: ./generator-oui/component/templates/_index.scss
  1. Pull Request [OSCI][CLEAN] Audit unused dependencies in OUI #1135 is created to incorporate the changes in points 1., 2. and 3.

Regards,

Samuel

@BSFishy
Copy link
Contributor Author

BSFishy commented Nov 7, 2023

Furthermore, I just found this package: https://www.npmjs.com/package/@vercel/nft, which is meant to identify files needed at runtime. I wrote a quick script to run this on OUI: https://gist.github.com/BSFishy/f917770acac228565e98d9d819d582e5 and got this output:

Set(15) {
  'react',
  'prop-types',
  'react-dom',
  'moment',
  'classnames',
  'react-beautiful-dnd',
  'scheduler',
  'object-assign',
  'use-memo-one',
  'css-box-model',
  'memoize-one',
  'raf-schd',
  'tiny-invariant',
  'symbol-observable',
  'hoist-non-react-statics'
}

Again, this isn't a catch-all and should be taken with a grain of salt, but this could be a good starting place for our dependencies list, allowing us to move everything else into devDependencies, reducing the necessary packages needed to download OUI as a dependency

@BigSamu
Copy link
Contributor

BigSamu commented Nov 8, 2023

Thanks @BSFishy. Let's organize ourselves to take a look to the runtime dependencies again.

@Willie-The-Lord, @JohnathonBowers,

Now I am having issues to run our branch, not sure why? I was working in other issues in other branches and when I got to this branch I did a start refresh as always running the following:

rm -rf node_modules yarn.lock
yarn clean
yarn install

Then I ran yarn test to check everything is in order and I got the following error:

...<More Lines to the Top>

137     expect(instance.isNodeOpen(items[1])).toBe(true);
                        ~~~~~~~~~~

src/components/tree_view/tree_view.test.tsx:139:21 - error TS2339: Property 'isNodeOpen' does not exist on type 'Component<{}, {}, any>'.

139     expect(instance.isNodeOpen(items[0])).toBe(true);
                        ~~~~~~~~~~

src/components/tree_view/tree_view.test.tsx:141:14 - error TS2339: Property 'handleNodeClick' does not exist on type 'Component<{}, {}, any>'.

141     instance.handleNodeClick(items[0]);
                 ~~~~~~~~~~~~~~~

src/components/tree_view/tree_view.test.tsx:142:21 - error TS2339: Property 'isNodeOpen' does not exist on type 'Component<{}, {}, any>'.

142     expect(instance.isNodeOpen(items[0])).toBe(false);
                        ~~~~~~~~~~

src/utils/deprecated/deprecated.tsx:36:15 - error TS2786: 'Component' cannot be used as a JSX component.
  Its element type 'ReactElement<any, any> | Component<any, any, any> | null' is not a valid JSX element.
    Type 'Component<any, any, any>' is not assignable to type 'Element | ElementClass | null'.
      Type 'Component<any, any, any>' is not assignable to type 'ElementClass'.
        The types returned by 'render()' are incompatible between these types.
          Type 'React.ReactNode' is not assignable to type 'import("/Users/samuel/Project_Code/OpenSearch_Initiative_Programme/oui/node_modules/@types/enzyme/node_modules/@types/react/ts5.0/index").ReactNode'.

36       return <Component {...props} />;
                 ~~~~~~~~~


Found 305 errors in 43 files.

Errors  Files
   176  node_modules/@types/enzyme/node_modules/@types/react/ts5.0/index.d.ts:3290
     6  node_modules/@types/react/index.d.ts:3127
     1  src-docs/src/components/guide_section/guide_section.tsx:186
     3  src-docs/src/components/guide_section/guide_section_parts/guide_section_tabs.tsx:99
     2  src-docs/src/views/page/_page_demo.tsx:123
    12  src/components/accessibility/keyboard_accessible.test.tsx:73
     1  src/components/accordion/accordion.test.tsx:219
     1  src/components/accordion/accordion.tsx:316
     2  src/components/basic_table/basic_table.tsx:749
     1  src/components/bottom_bar/bottom_bar.tsx:233
     5  src/components/combo_box/combo_box.test.tsx:444
     1  src/components/combo_box/combo_box.tsx:1010
     1  src/components/context_menu/context_menu_panel.tsx:539
     1  src/components/control_bar/control_bar.tsx:475
     1  src/components/copy/copy.test.tsx:63
     1  src/components/datagrid/data_grid_body.tsx:691
     1  src/components/datagrid/data_grid_cell.tsx:171
     1  src/components/datagrid/data_grid_cell_buttons.tsx:95
     3  src/components/datagrid/data_grid_cell_popover.tsx:94
     1  src/components/datagrid/data_grid_control_header_cell.tsx:201
     2  src/components/datagrid/data_grid_inmemory_renderer.tsx:98
     2  src/components/date_picker/date_picker.tsx:241
    16  src/components/date_picker/super_date_picker/super_date_picker.test.tsx:55
     5  src/components/delay_hide/delay_hide.test.tsx:39
     2  src/components/error_boundary/error_boundary.test.tsx:50
     1  src/components/flex/flex_grid.tsx:126
     1  src/components/flex/flex_item.tsx:83
     2  src/components/flyout/flyout.tsx:394
     1  src/components/focus_trap/focus_trap.test.tsx:219
     1  src/components/icon/icon.tsx:811
     1  src/components/markdown_editor/markdown_editor.tsx:473
     1  src/components/observer/mutation_observer/mutation_observer.test.tsx:48
     1  src/components/observer/resize_observer/resize_observer.test.tsx:73
     4  src/components/outside_click_detector/outside_click_detector.test.tsx:43
     1  src/components/popover/input_popover.tsx:133
     3  src/components/popover/popover.tsx:786
     1  src/components/popover/wrapping_popover.tsx:74
     1  src/components/portal/portal.test.tsx:39
    26  src/components/resizable_container/resizable_container.test.tsx:43
     1  src/components/text_diff/text_diff.tsx:97
     2  src/components/tool_tip/tool_tip.tsx:333
     8  src/components/tree_view/tree_view.test.tsx:112
     1  src/utils/deprecated/deprecated.tsx:36
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Any suggestions what could be?

Regards,

Samuel

@BigSamu
Copy link
Contributor

BigSamu commented Nov 8, 2023

Ok, I found the problem. It happens when you remove yarn.lock and then is created again when you run yarn install. But this is weird. @BSFishy, shouldn't package.json have to have all the required information to create the lock file + the node modules? I guess there is a problem in package.json where dependencies versions are details with ^<version>, meaning that they can use the detailed version or a greater one. But maybe that could bring problems when creating the dependency tree relations in the lock file, with dependencies that don't work together with some greater versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed refactor Research An open request for eng to research one or more topics
Projects
Status: Todo
Development

No branches or pull requests

5 participants