Update node dependencies #1596
Update node dependencies #1596
Conversation
ahangarha
force-pushed
the
update-node-dependencies
branch
from
7223023 to
326033c
Compare
|
please rebase and update |
|
@justin808
|
This commit causes failing tests. This is made to check the failures.
ahangarha
force-pushed
the
update-node-dependencies
branch
from
c9958f1 to
f9e76fd
Compare
| /* eslint-disable @typescript-eslint/no-explicit-any */ | ||
|
|
There was a problem hiding this comment.
This part needs to be improved. I couldn't find a way other than ignoring the linter rule.
There was a problem hiding this comment.
Some times there's no better way to do things than to ignore linter rules.
That's because some linter rules are just for identifying possible code smells.
| const actual = ReactOnRails.render('R1', {}, 'root')._reactInternalFiber.type; | ||
| const actual = ReactOnRails.render('R1', {}, 'root')._reactInternals.type; |
There was a problem hiding this comment.
This is internal logic in React, and there is no documentation about it. But in React 17, ReactOnRails.render('R1', {}, 'root') only has ._reactInternals In React 18, this is changed again.
ahangarha
force-pushed
the
update-node-dependencies
branch
from
0da8f1c to
29d6053
Compare
ahangarha
force-pushed
the
update-node-dependencies
branch
from
248abdf to
060165f
Compare
js-dom version 23 requires Node 18
This reverts commit b6890f4.
| @@ -51,7 +51,7 @@ const configLoader = (configPath) => { | |||
| // Some test environments might not have the NODE_ENV set, so we'll have fallbacks. | |||
| const configEnv = env.NODE_ENV || env.RAILS_ENV || 'development'; | |||
| const ymlConfigPath = join(configPath, 'webpacker.yml'); | |||
| const settings = safeLoad(readFileSync(ymlConfigPath, 'utf8'))[configEnv]; | |||
| const settings = load(readFileSync(ymlConfigPath, 'utf8'))[configEnv]; | |||
There was a problem hiding this comment.
@ahangarha can you explain why we had to stop using safeLoad?
There was a problem hiding this comment.
Sure. That is because of the new changes in js-yaml v4.0.0.
Breaking: removed
safe*functions. Useload,loadAll,dumpinstead which are all now safe by default.
They are all safe by this version.
There was a problem hiding this comment.
Figure out why we still have this file and if we can delete this.
There was a problem hiding this comment.
We don't use this file anymore, but I am not sure if no other project uses this as config loader. It is very unlikely but still...
It is included in out npm package and removing it is a breaking change.
What about putting this file in the queue for removal in the next major release and for now, we show a deprecation message for using it?
| "react": ">= 17", | ||
| "react-dom": ">= 17" |
There was a problem hiding this comment.
@Judahmeek @ahangarha can we under the bump of React?
There was a problem hiding this comment.
Currently, no logic has changed in the codebase regarding React API (except in tests, which is not our concern here).
I think we can keep the minimum version of React to 16.
We drop support for Node v14 and v17 because of bumping
@typescript-eslint/parserversion to 6+ and js-dom to v22Remaining upgrades:
eslint-config-shakacodeCan we do these upgrades in another PR?
This change is