-
Notifications
You must be signed in to change notification settings - Fork 13
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
[PACKAGE] : Updates ⤴️ project dependencies 📦 #189
Conversation
package.json
Outdated
"styled-components": "^6.1.8", | ||
"stylelint-config-standard-scss": "^13.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
styled-components
dropping console warnings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All other dependencies appear to be in good order, but styled-components
is causing some errors and warnings in the development console:
styled-components: It appears that an unknown prop "level" is being sent to the DOM, which may trigger a React console error. If you wish to automatically filter out unknown props, you can opt into that behavior via `<StyleSheetManager shouldForwardProp={...}>` (by connecting an API like `@emotion/is-prop-valid`) or consider using transient props (prefix with `$` for automatic filtering).
Additionally, there are warnings such as:
Warning: React does not recognize the `marginRight` prop on a DOM element. If you intentionally want it to appear in the DOM as a custom attribute, spell it as lowercase `marginright` instead. If you accidentally passed it from a parent component, remove it from the DOM element.
in div (created by styled.div)
in styled.div (at AssetNode.js:102)
in div (created by styled.div)
in styled.div (at AssetNode.js:81)
in AssetNode (at AssetTree.js:89)
in div (at AssetTree.js:103)
in AssetTree (at Assets.js:375)
in div (at Assets.js:338)
in div (at Assets.js:400)
in assetsComponent (at Project.js:624)
in div (created by MuiTabPanelRoot)
in MuiTabPanelRoot (created by ForwardRef(TabPanel))
in ForwardRef(TabPanel) (at Project.js:715)
in TabContext (at Project.js:683)
in div (at Project.js:734)
in Project (created by WithStyles(Project))
in WithStyles(Project) (at ProjectPage.js:315)
in div
in div
in t (at ProjectPage.js:293)
in div (at ProjectPage.js:292)
in ProjectPage (created by Context.Consumer)
in Route (at Routes.js:18)
in Switch (at Routes.js:14)
in ThemeProvider (created by ThemeProvider)
in ThemeProvider (created by ThemeProvider)
in ThemeProvider (at App.js:174)
in App (at Routes.js:13)
in Router (created by HashRouter)
in HashRouter (at Routes.js:12)
in _default (at Root.js:5)
in Root (created by HotExportedRoot)
in AppContainer (created by HotExportedRoot)
in HotExportedRoot
in AppContainer (at app/index.js:10)
These issues are occurring in various components of the application.
Could you please look into this? Also, please update the list of dependencies you are updating in the associated Issue by adding a comment. As per the issue instructions, it's better to do this whenever you are working on dependencies, so if someone else is working, they will avoid updating those.
Looking into it. |
v5/v6 docs for styled-components call for using transient props to avoid these console errors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It resolves it for AssetNode
, but there are still some errors in PeopleTable
, ProjectNotes
, and ProjectLog
.
Steps to reproduce:
- Open Statwrap using
yarn dev
. - Navigate to the
notes
orproject log
under a selected project. - Check the dev console.
Once you resolve the errors, try to reload the application and redo the steps in order to check for leftover errors.
Yeah, I of course know these things, I wasn't finished with the changes. Was quite busy with something else for past few days, I'll continue with it today. |
Okay @AdiAkhileshSingh15, I thought you might not be aware of this as you pushed some new commits. Also, I want to mention one more thing: could you please try to resolve these dependencies too - |
Sure, I'll look into these as well, that'd be helpful for sure to be left with only major React updates. |
thanks 👍 |
Updating this Though, it helps to remove those specific console errors as we update the react-data-table-components, it brings up a new error "TypeError: t is not a function", which am unable to find a solution to, yet. Suspecting it's v17 react that's causing this error, though not sure. Will update here, if I get a breakthrough, otherwise we might have to work with v6 of |
Update: For |
Thanks for the update @AdiAkhileshSingh15, also have you looked into |
I tried that back ago, it had some backward-incompatible changes. |
for @mui/lab as well ? |
Yes, it showed some "ENOENT: no such file or directory" errors, last time checked. Still, let me retry and resolve it. |
Resolved 🕺🏻 |
These are the latest dependencies with non-breaking changes, for our project. @lrasmus please review the changes. |
Updated the changes summary with latest modifications 👍🏻 . |
will review the PR soon 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @AdiAkhileshSingh15 I've reviewed all the changes, and they look good to me overall. However, it seems that remark-gfm
is dropping some errors in its functionality. :
StatWrap.2024-03-18.22-13-32.mp4
could you please look into this ? Also I think we can revert the changes for app/components/AssetTree/AssetNode/AssetNode.js
file, as these changes might no longer be needed since we are not updating to version 6 of styled-components
package.json
Outdated
"remark-gfm": "^4.0.0", | ||
"resizable-panels-react": "^2.0.8", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issue around remark-gfm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will look into it soon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0b1ccbc1-c607-4bec-8e70-c7bdfb557ecb.mp4
Video demo for remark-gfm package older version, it used to function similarly. The strikethrough in this case is only enabled if there are equal no. of '~' on both sides
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But the errors are not related solely to strikethrough functionality. I tested it with the features listed here and encountered some errors (https://www.npmjs.com/package/remark-gfm), which were not present in version 1 but appeared in version 4, the above video I shared not about a single functionality but about the console errors it drops and for me it also crashing the application sometimes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Abhijay007 Got to know that the latest remark-gfm v4 only works with latest react-markdown , i.e., v9, so, I'm downgrading remark-gfm to v1 only, till we don't upgrade react-markdown, along with other react packages, which have some breaking changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @AdiAkhileshSingh15 for looking into this. Two more things I want to mention:
-
Please run
yarn upgrade-interactive --latest
for the latest results and upgrade the dependencies which have minor changes. I noticed you did this for most of them, but I believe there are a few more left. -
Secondly, please squash your commits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
v2 and v3 are bringing in some issues with missing modules, which we face while upgrading react-markdown as well, 'unified' module not found.
So, I believe the gfm upgrade can be performed parallely with react-markdown, we have to keep this in mind.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so it is better to downgrade remark-gfm
to version one for now
flex-direction: row; | ||
align-items: center; | ||
padding: 5px 8px; | ||
padding-left: ${props => getPaddingLeft(props.level)}px; | ||
padding-left: ${props => getPaddingLeft(props.$level)}px; | ||
${props => (props.selected ? 'background-color: #eee;' : null)} | ||
`; | ||
|
||
const NodeIcon = styled.div` | ||
font-size: 12px; | ||
margin-right: ${props => (props.marginRight ? props.marginRight : 5)}px; | ||
margin-right: ${props => (props.$marginright ? props.$marginright : 5)}px; | ||
`; | ||
|
||
const StyledInput = styled.input` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changes in file app/components/AssetTree/AssetNode/AssetNode.js
can be reverted as we are not using style-components
version 6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, actually this transient props thing came with v5 only. That is why I kept these changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, thank you for the information. We can keep it then, I have yet to test it with version 5, but I believe it might not be causing any errors as you might have already tested these changes.
e6f74ac
to
94c56c2
Compare
Hi @AdiAkhileshSingh15, if you run the latest |
Okay, I'll do it. |
fabf6fc
to
8a5617a
Compare
I've documented the changes in the summary. |
@AdiAkhileshSingh15 it seems like you missed my comments here :#189 (comment) , please consider these changes if you find them relevant |
@lrasmus |
Hi @AdiAkhileshSingh15, if you run the latest |
@AdiAkhileshSingh15 - this is looking good for me! Could you do the minor bumps per the last comment from @Abhijay007 , and then I can merge? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looking good - previous issue I ran into is gone.
package.json
Outdated
"remark-gfm": "^4.0.0", | ||
"resizable-panels-react": "^2.0.8", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lrasmus I have updated the PR with the most recent commit. No console/test/build or dev errors were detected. |
I've updated the changes summary. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
98e9b5a
to
56ce09f
Compare
merge conflicts resolved, thanks @AdiAkhileshSingh15, could you please squash these commits too? |
Yeah, was onto it 😂 |
commit 56ce09f Author: AdiAkhileshSingh15 <[email protected]> Date: Mon Mar 25 21:59:25 2024 +0530 updates dependencies related to babel, electron, stylelint, webpack, winston, and postcss commit 9dfda88 Author: AdiAkhileshSingh15 <[email protected]> Date: Wed Mar 20 13:24:51 2024 +0530 updates webpack-dev-server commit d7ff817 Author: AdiAkhileshSingh15 <[email protected]> Date: Wed Mar 20 13:16:53 2024 +0530 downgrades remark-gfm to bring in line with react-markdown commit 4b4efa7 Author: AdiAkhileshSingh15 <[email protected]> Date: Wed Mar 20 00:33:53 2024 +0530 updates dependencies related to @babel, @mui, core-js & postcss commit aca9c54 Author: AdiAkhileshSingh15 <[email protected]> Date: Mon Mar 18 17:25:27 2024 +0530 updates @mui/lab with an import fix commit bbe8f11 Author: AdiAkhileshSingh15 <[email protected]> Date: Mon Mar 18 16:51:58 2024 +0530 few nonbreaking dependency version modifications commit 7d23b5a Author: AdiAkhileshSingh15 <[email protected]> Date: Sat Mar 16 17:42:29 2024 +0530 fixes console warnings with transient lowercase props commit 1f9f37c Author: AdiAkhileshSingh15 <[email protected]> Date: Sat Mar 16 12:51:16 2024 +0530 updates few non-breaking dependencies commit dc9a491 Author: AdiAkhileshSingh15 <[email protected]> Date: Sat Mar 16 01:13:22 2024 +0530 updates few dependencies d3, remark-gfm, styled-components
56ce09f
to
5b1144e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ready for merge, thanks for this PR 🚀
Anytime 😉 |
Thank you both! |
Changes Summary
This PR aims to update he existing dependencies to the latest LTS (Long Term Support) version.
The following dependencies have been updated:
devDependencies:
^7.24.3
^7.24.1
^7.24.1
^7.24.1
^7.24.1
^7.24.1
^7.24.1
^7.24.1
^7.24.1
^7.24.1
^7.24.1
^7.24.1
^7.24.3
^7.24.1
^7.24.1
29.1.5
^7.34.1
^16.3.0
^5.91.0
^5.0.4
dependencies:
^5.15.14
^5.0.0-alpha.169
^5.15.14
^5.15.14
^1.6.8
^3.36.1
7.9.0
8.4.38
^5.3.11
^3.13.0
All the dependencies are updated to their latest LTS version, except
styled-components
, and their were no breaking-changes detected in any of them, still I'd recommend everyone to reinstall the node_modules while locally performing the updates.Related Issue
In-progress: #171
Testing Checklist ✅ :
After updating dependencies, ran different checks using these commands locally to ensure functionality remains intact.
Use commands :
yarn test
- all tests passedyarn dev
- no UI fails detectedyarn build
- no build fails detectedyarn dev
- no new consol errors detected for any dependenciesReviewer(s)
@lrasmus