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.

Sign up for GitHub

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

Update node dependencies #1596

Merged
merged 16 commits into from
Jan 15, 2024
Merged

Update node dependencies #1596

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
eea3343
Bump babel packages version
ahangarha Jan 10, 2024
7d5f5e9
Bump Jest packages version
ahangarha Jan 10, 2024
b6473b2
Bump TypeScript to v5
ahangarha Jan 10, 2024
48ffe3f
Fix linter issues
ahangarha Jan 10, 2024
06cf069
Bump min version of Node to 16 in CI
ahangarha Jan 10, 2024
f9e76fd
Bump React version from 16 to 18
ahangarha Jan 12, 2024
8caf966
Downgrade React to v17
ahangarha Jan 13, 2024
4611093
Use reactInternals instead of reactInternalFiber
ahangarha Jan 13, 2024
29d6053
Bump js-yaml peer dependency to v4
ahangarha Jan 13, 2024
8adcaf8
Bump React peer dependency min version to v17
ahangarha Jan 13, 2024
060165f
Bump js-dom version to 23.2.0
ahangarha Jan 13, 2024
cf12f4a
Downgrade js-dom to v22
ahangarha Jan 13, 2024
1f22f99
Update changelog
ahangarha Jan 13, 2024
b6890f4
Update yarn lock file
ahangarha Jan 13, 2024
bfd7d31
Revert "Update yarn lock file"
ahangarha Jan 13, 2024
1065f3f
Bump minor versions for dev dependencies
ahangarha Jan 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ jobs:
strategy:
matrix:
ruby: [3.0, 3.3]
node: [14, 20]
node: [16, 20]
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v4
Expand Down Expand Up @@ -84,7 +84,7 @@ jobs:
strategy:
matrix:
ruby: [3.0, 3.3]
node: [14, 20]
node: [16, 20]
rake_task: ['run_rspec:all_but_examples', 'run_rspec:examples']
runs-on: ubuntu-22.04
steps:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/package-js-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ jobs:
build:
strategy:
matrix:
node: [14, 20]
node: [16, 20]
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v4
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/rspec-package-specs.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ jobs:
strategy:
matrix:
ruby: [3.0, 3.3]
node: [14, 20]
node: [16, 20]
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v4
Expand Down
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,14 @@ Please follow the recommendations outlined at [keepachangelog.com](http://keepac
### [Unreleased]
Changes since the last non-beta release.

#### Changed
- Bump minimum version of peer dependencies as following [PR 1596](https://github.com/shakacode/react_on_rails/pull/1596) by [ahangarha](https://github.com/ahangarha).
- `js-yaml` to `4.1.0`
- `react` and `react-dom` to `17.0.0`

#### Removed
- Dropped Ruby 2.7 support [PR 1595](https://github.com/shakacode/react_on_rails/pull/1595) by [ahangarha](https://github.com/ahangarha).
- Dropped support for Node 14 and 17 [PR 1596](https://github.com/shakacode/react_on_rails/pull/1596) by [ahangarha](https://github.com/ahangarha).

### [13.4.1]
#### Fixed
Expand Down
2 changes: 2 additions & 0 deletions node_package/src/buildConsoleReplay.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import RenderUtils from './RenderUtils';
import scriptSanitizedVal from './scriptSanitizedVal';

/* eslint-disable @typescript-eslint/no-explicit-any */

Comment on lines +4 to +5
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part needs to be improved. I couldn't find a way other than ignoring the linter rule.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

declare global {
interface Console {
history?: {
Expand Down
3 changes: 2 additions & 1 deletion node_package/src/clientStartup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import { isServerRenderHash } from './isServerRenderResult';
import reactHydrateOrRender from './reactHydrateOrRender';
import { supportsRootApi } from './reactApis';

/* eslint-disable @typescript-eslint/no-explicit-any */

declare global {
interface Window {
ReactOnRails: ReactOnRailsType;
Expand Down Expand Up @@ -302,7 +304,6 @@ export function clientStartup(context: Context): void {
if (!isWindow(context)) {
return;
}
const { document } = context;

// Tried with a file local variable, but the install handler gets called twice.
// eslint-disable-next-line no-underscore-dangle
Expand Down
2 changes: 1 addition & 1 deletion node_package/src/reactApis.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import ReactDOM from 'react-dom';

const reactMajorVersion = ReactDOM.version?.split('.')[0] || 16;
const reactMajorVersion = Number(ReactDOM.version?.split('.')[0]) || 16;

// TODO: once we require React 18, we can remove this and inline everything guarded by it.
// Not the default export because others may be added for future React versions.
Expand Down
2 changes: 2 additions & 0 deletions node_package/src/serverRenderReactComponent.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import buildConsoleReplay from './buildConsoleReplay';
import handleError from './handleError';
import type { RenderParams, RenderResult, RenderingError } from './types/index';

/* eslint-disable @typescript-eslint/no-explicit-any */

function serverRenderReactComponentInternal(options: RenderParams): null | string | Promise<RenderResult> {
const { name, domNodeId, trace, props, railsContext, renderingReturnsPromises, throwJsErrors } = options;

Expand Down
2 changes: 1 addition & 1 deletion node_package/tests/ReactOnRails.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe('ReactOnRails', () => {

document.body.innerHTML = '<div id="root"></div>';
// eslint-disable-next-line no-underscore-dangle
const actual = ReactOnRails.render('R1', {}, 'root')._reactInternalFiber.type;
const actual = ReactOnRails.render('R1', {}, 'root')._reactInternals.type;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

expect(actual).toEqual(R1);
});

Expand Down
46 changes: 23 additions & 23 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,45 +14,45 @@
"@babel/preset-env": "^7.20.2",
"@babel/preset-react": "^7.18.6",
"@babel/types": "^7.20.7",
"@types/jest": "^28.1.3",
"@types/react": "^16.14.35",
"@types/react-dom": "^16.9.17",
"@types/turbolinks": "^5.2.0",
"@types/webpack-env": "^1.18.0",
"@typescript-eslint/eslint-plugin": "^4.33.0",
"@typescript-eslint/parser": "^4.33.0",
"@types/jest": "^29.0.0",
"@types/react": "^17.0.0",
"@types/react-dom": "^17.0.0",
"@types/turbolinks": "^5.2.2",
"@types/webpack-env": "^1.18.4",
"@typescript-eslint/eslint-plugin": "^6.18.1",
"@typescript-eslint/parser": "^6.18.1",
"babelify": "^10.0.0",
"blue-tape": "^1.0.0",
"concurrently": "^8.2.0",
"concurrently": "^8.2.2",
"create-react-class": "^15.7.0",
"eslint": "^7.32.0",
"eslint-config-prettier": "^7.0.0",
"eslint-config-shakacode": "^16.0.1",
"eslint-plugin-import": "^2.27.5",
"eslint-plugin-jsx-a11y": "^6.7.1",
"eslint-plugin-import": "^2.29.1",
"eslint-plugin-jsx-a11y": "^6.8.0",
"eslint-plugin-prettier": "^3.4.1",
"eslint-plugin-react": "^7.32.1",
"jest": "^28.1.3",
"jest-environment-jsdom": "^28.1.3",
"jsdom": "^16.4.0",
"eslint-plugin-react": "^7.33.2",
"jest": "^29.0.0",
"jest-environment-jsdom": "^29.0.0",
"jsdom": "^22.1.0",
"nps": "^5.9.3",
"prettier": "^2.8.3",
"prettier": "^2.8.8",
"prettier-eslint-cli": "^5.0.0",
"prop-types": "^15.8.1",
"react": "^16.14.0",
"react-dom": "^16.14.0",
"react": "^17.0.0",
"react-dom": "^17.0.0",
"react-transform-hmr": "^1.0.4",
"redux": "^4.2.0",
"ts-jest": "^28.0.8",
"typescript": "^4.9.4"
"redux": "^4.2.1",
"ts-jest": "^29.1.0",
"typescript": "^5.3.3"
},
"dependencies": {
"@babel/runtime-corejs3": "^7.12.5"
},
"peerDependencies": {
"js-yaml": ">= 3.0.0",
"react": ">= 16",
"react-dom": ">= 16"
"js-yaml": ">= 4.1.0",
"react": ">= 17",
"react-dom": ">= 17"
Comment on lines +54 to +55
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Judahmeek @ahangarha can we under the bump of React?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

},
"files": [
"node_package/lib",
Expand Down
4 changes: 2 additions & 2 deletions webpackConfigLoader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

const { join, resolve } = require('path');
const { env } = require('process');
const { safeLoad } = require('js-yaml');
const { load } = require('js-yaml');
const { readFileSync } = require('fs');

function removeOuterSlashes(string) {
Expand Down Expand Up @@ -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];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ahangarha can you explain why we had to stop using safeLoad?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. That is because of the new changes in js-yaml v4.0.0.

Breaking: removed safe* functions. Use load, loadAll, dump instead which are all now safe by default.

They are all safe by this version.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Figure out why we still have this file and if we can delete this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@justin808 @Judahmeek

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?


// NOTE: Rails path is hard coded to `/public`
const output = {
Expand Down
Loading
Loading