Skip to content

Add support for react 19 #7532

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

Merged
merged 6 commits into from
Apr 15, 2025
Merged

Add support for react 19 #7532

merged 6 commits into from
Apr 15, 2025

Conversation

jcfilben
Copy link
Collaborator

@jcfilben jcfilben commented Feb 27, 2025

What does this PR do?

Add support for react 19
https://react.dev/blog/2024/04/25/react-19-upgrade-guide

I replaced places where we used JSX.IntrinsicElements['div'] with React.DetailedHTMLProps<React.HTMLAttributes<HTMLDivElement>, HTMLDivElement> because in React 19 the JSX namespace moved to be under React rather than being global.

React now gives an error when an empty string is passed to src so I changed some of the tests to resolve this.

useRef() now requires a parameter, so there are some changes related to adding parameters to places where perviously we had none.

the inert prop is now recognized in react 19, so it can receive a value of true or false, whereas previously we needed to pass an empty string to set inert to true. I created the inertTrueValue variable to handle the difference between react 19 and react <18.

I made a few changes to the Tip component to manage the changes to refs that were made in React 19. (More details: https://react.dev/blog/2024/12/05/react-19#ref-as-a-prop)

I came across this issue that mentions potential performance issues with forwardRef in react 19. It might be good to test React 19 + this branch of Grommet in a large project just to see if there are any performance differences. facebook/react#31613

Where should the reviewer start?

What testing has been done on this PR?

Tested this branch with React 16, 17, 18, and 19 and confirmed they are all working.
I also did performance testing, see screenshots below.

How should this be manually tested?

Do Jest tests follow these best practices?

  • screen is used for querying.
  • The correct query is used. (Refer to this list of queries)
  • asFragment() is used for snapshot testing.

Any background context you want to provide?

What are the relevant issues?

Closes #7458

Screenshots (if appropriate)

Performance testing on app with components from 'all' story

React 19 + this branch

average = 461.5

"timestamp": 490.4,
Screenshot 2025-03-03 at 12 52 25 PM
"timestamp": 521.5,
Screenshot 2025-03-03 at 12 52 41 PM
"timestamp": 418.5,
Screenshot 2025-03-03 at 12 52 53 PM
"timestamp": 447.7
Screenshot 2025-03-03 at 12 53 07 PM
"timestamp": 429.5,
Screenshot 2025-03-03 at 12 53 17 PM

React 18 + this branch

average = 499.1

"timestamp": 484.3,
Screenshot 2025-03-03 at 12 53 28 PM
"timestamp": 478.5,
Screenshot 2025-03-03 at 12 59 15 PM
"timestamp": 529.1,
Screenshot 2025-03-03 at 12 59 27 PM
"timestamp": 504.3,
Screenshot 2025-03-03 at 12 59 38 PM
"timestamp": 499.2,
Screenshot 2025-03-03 at 12 59 51 PM

React 18 + master

average = 531.7

"timestamp": 526.3,
Screenshot 2025-03-03 at 2 17 36 PM
"timestamp": 463.2,
Screenshot 2025-03-03 at 2 17 44 PM
"timestamp": 592.9,
Screenshot 2025-03-03 at 2 18 01 PM
"timestamp": 519.8,
Screenshot 2025-03-03 at 2 18 11 PM
"timestamp": 556.1,
Screenshot 2025-03-03 at 2 18 21 PM

Do the grommet docs need to be updated?

no

Should this PR be mentioned in the release notes?

yes

Is this change backwards compatible or is it a breaking change?

backwards compatible

@@ -91,20 +91,22 @@ const Text = forwardRef(
</StyledText>
);

const tipProps = tipProp && typeof tipProp === 'object' ? tipProp : {};
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems odd to me. If tipProp is defined but not an object why would we do {}? It seems like that loses tipProp.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

tipProp is either a string (which will be used for the content of the tooltip) or an object. If tipProp is a string then it doesn't make sense to pass that string as a prop to the Tooltip component. It should instead be passed as the child or through the content prop. This is why I set tipProps to an empty object if it is not an object.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That makes sense. Although there's also a pre-existing bug here. tip="some string content" doesn't work (previous version or this version.) This is what you get on hover with:

<Text tip="Some cool tip">Text with string tip</Text>

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh interesting, good callout

@jcfilben jcfilben requested a review from MikeKingdom March 7, 2025 15:40
@britt6612
Copy link
Collaborator

I was able to get our DS site upgraded to react 19 then pointing to this PR. Im looking through pages now make sure everything is running smooth so far so good.. but will update once Im finished

Copy link
Collaborator

@britt6612 britt6612 left a comment

Choose a reason for hiding this comment

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

From DS site performance

Using React 19 & this branch

First
Screenshot 2025-03-11 at 1 11 35 PM

"timestamp": 1479.8999999761581,

Second
Screenshot 2025-03-11 at 1 17 44 PM

     "timestamp": 1467.300000011921,

Third
Screenshot 2025-03-11 at 1 19 27 PM

      "timestamp": 1495.199999988079,

Using React 18 & this branch

First
Screenshot 2025-03-11 at 1 23 30 PM

"timestamp": 1718.6000000238419,

Second
Screenshot 2025-03-11 at 1 24 49 PM
"timestamp": 1650.6000000238419,

Third
Screenshot 2025-03-11 at 1 25 21 PM
"timestamp": 1636.5,

@halocline halocline self-requested a review April 9, 2025 23:18
@halocline
Copy link
Collaborator

halocline commented Apr 14, 2025

@jcfilben - Proposing the following summary for the React production mode test results:

React19 Upgrade Performance Summary

Overview

Grommet would like to to support React19. In doing so, the following evaluation
was conducted to ensure that there is no performance degradation to Grommet-based
projects when supporting React19.

Summary findings

  • Performance improved by 8% for the production build of Grommet + React19.
    • For the test application, React19 reduced render duration from a mean of 2660
      milliseconds (ms) to 2446 ms (-214 ms, -8.0%).
  • Dev mode profiles were not used in the performance evaluation due to React 19 running
    slower in dev mode.
  • Conclusion: Grommet + React 19 saw no performance degradation, so recommendation is
    to move forward supporting React 19 in Grommet.

Methodology

  • Use React DevTools Profiler timings to compare load times for a common application.
  • Application:
    • Built with React (v18.3.1 & v19.1.0) + Grommet (v2.46.0) + TypeScript + Vite.
    • Application consumed 1,000+ record data set, displaying the data set in three
      DataCharts and one DataTable.
  • Tests:
    • Grommet + React 18 + Vite production mode
    • Grommet + React 19 + Vite production mode
  • Each test:
    • 20 samples.
    • Ran React Profiler to record the total render tree duration for the application.
    • Calculated summary duration statistics:
      • min
      • max
      • mean
      • median
      • mode
      • variance
      • standard deviation
      • range

Results

Duration React 18 React 19
Sample size 20 20
Mean (ms) 2660 2446
Min (ms) 2590 2331
Max (ms) 2786 2663
StdDev (ms) 59 65

React 18

{
  "stats": {
    "total": 53197.299999952316,
    "reactVersion": "18.3.1-next-f1338f8080-20240426",
    "durations": [
      2589.899999976158,
      2592.100000023842,
      2605.399999976158,
      2613.5,
      2614.099999964237,
      2615.800000011921,
      2617,
      2617.599999964237,
      2622.300000011921,
      2632.300000011921,
      2635.300000011921,
      2660.100000023842,
      2665.199999988079,
      2671.600000023842,
      2687.800000011921,
      2721.599999964237,
      2739.399999976158,
      2755.300000011921,
      2755.5,
      2785.5
    ],
    "min": 2589.899999976158,
    "max": 2785.5,
    "mean": 2659.864999997616,
    "median": 2635.300000011921,
    "variance": 3507.965275049305,
    "stdDev": 59.22807843455083,
    "range": 195.60000002384186,
    "mode": [
      "2620",
      4
    ]
  }
}

React 19

{
  "stats": {
    "total": 48923.099999998696,
    "reactVersion": "19.1.0",
    "durations": [
      2330.7999999998137,
      2405.5999999996275,
      2406.5,
      2413.0999999996275,
      2415.4000000003725,
      2416.199999999255,
      2417.699999999255,
      2419,
      2422,
      2424.4000000003725,
      2430.5,
      2439.2000000001863,
      2440.4000000003725,
      2459.4000000003725,
      2461.7999999998137,
      2464.899999999441,
      2465.4000000003725,
      2465.5,
      2562.2999999998137,
      2663
    ],
    "min": 2330.7999999998137,
    "max": 2663,
    "mean": 2446.1549999999347,
    "median": 2430.5,
    "variance": 4220.889475004651,
    "stdDev": 64.96837288253917,
    "range": 332.20000000018626,
    "mode": [
      "2420",
      6
    ]
  }
}

Resources

React 19 slower in dev mode

React19 runs slower in dev mode due to several factors introduced to improve
debugging and developer experience. Some of the primary reasons include:

  • Strict mode enhancements - React 19 has enhanced Strict Mode to catch more
    potential issues in the app, such as unsafe lifecycle methods, deprecated APIs,
    and unintentional side effects. In development mode, this leads to double rendering
    for some components to detect issues. While this makes the app safer, it increases
    the amount of rendering React has to do, which can make it feel slower in development.
  • Profiling and Debugging Overhead - React 19 may have introduced more profiling
    tools and logging mechanisms in development. These tools help developers catch issues
    early but introduce additional performance overhead, such as more frequent checks and
    more verbose logging. This can make React apps feel slower while in development.
  • Larger DevTools and Hot-Reloading - With updates to React DevTools, hot-reloading,
    and debugging tools, React 19 may require more resources to manage the state of the app.
    These tools are crucial for providing real-time feedback during development but can also
    impact performance due to the extra work of maintaining the state of the app, especially
    when hot-reloading changes.

Summary script

// performance-benchmark.mjs
//
// From directory containing React DevTools Profiler JSON results,
// reads each JSON profile, summarizes, and writes summary stats to file.

import { argv } from 'node:process';
import { join } from 'node:path';
import { existsSync, promises as fsPromises } from 'node:fs';
import { parseArgs } from 'node:util';

const args = argv.slice(2);

const options = {
  input: {
    type: 'string',
    short: 'i',
    isRequired: true,
  },
  output: {
    type: 'string',
    short: 'o',
    isRequired: true,
  },
};

const { values } = parseArgs({
  args,
  options,
  tokens: true,
});

const inputDirectory = values.input;
// Get parts from the input directory path
const inputDirectoryParts = inputDirectory
  .split('/')
  .filter((part) => part !== '');
const inputDirectoryName = inputDirectoryParts[inputDirectoryParts.length - 1];

const outputFile = values.output || `${inputDirectoryName}-results.json`;
const outputDirectory = join(inputDirectory, 'output');

// Check if the input directory exists
if (!existsSync(inputDirectory)) {
  console.error(`Input directory "${inputDirectory}" does not exist.`);
  process.exit(1);
}

// Read each file in the input directory
const files = await fsPromises.readdir(inputDirectory, { withFileTypes: true });
const inputFiles = files
  .filter((file) => file.isFile() && file.name.endsWith('.json'))
  .map((file) => file.name);

// Read each file and parse the JSON
const snapshots = await Promise.all(
  inputFiles.map(async (file) => {
    const adjFile = join(inputDirectory, file);
    try {
      const data = await fsPromises.readFile(adjFile, 'utf8');
      const jsonObject = JSON.parse(data);
      return jsonObject;
    } catch (err) {
      console.error(`Error processing file "${file}":`, err);
      return null;
    }
  }),
);

const stats = snapshots
  .map((snapshot) => {
    const { timelineData } = snapshot;
    const { duration, reactVersion } = timelineData[0];
    return {
      duration,
      reactVersion,
    };
  })
  .sort((a, b) => a.duration - b.duration)
  .reduce(
    (acc, curr) => {
      acc.total += curr.duration;
      acc.durations.push(curr.duration);
      acc.reactVersion = curr.reactVersion;
      return acc;
    },
    {
      total: 0,
      reactVersion: '',
      durations: [],
    },
  );

stats.min = Math.min(...stats.durations);
stats.max = Math.max(...stats.durations);
stats.mean = stats.total / stats.durations.length;
stats.median = stats.durations[Math.floor(stats.durations.length / 2)];
stats.variance =
  stats.durations.reduce((acc, curr) => acc + (curr - stats.mean) ** 2, 0) /
  stats.durations.length;
stats.stdDev = Math.sqrt(stats.variance);
stats.range = stats.max - stats.min;
stats.mode = stats.durations.reduce((acc, curr) => {
  const roundCurr = Math.round(curr / 10) * 10;
  acc[roundCurr] = (acc[roundCurr] || 0) + 1;
  return acc;
}, {});
stats.mode = Object.entries(stats.mode).reduce(
  (acc, curr) => {
    if (curr[1] > acc[1]) {
      return curr;
    }
    return acc;
  },
  [0, 0],
);

console.log('Stats:', stats);

// Write the stats to the output file
const outputData = {
  stats,
};
const outputDirectoryExists = existsSync(outputDirectory);
if (!outputDirectoryExists) {
  await fsPromises.mkdir(outputDirectory, { recursive: true });
}

const outputFilePath = join(outputDirectory, outputFile);
const outputFileExists = existsSync(outputFilePath);
if (outputFileExists) {
  console.log(`Output file "${outputFilePath}" already exists. Overwriting...`);
}
await fsPromises.writeFile(
  outputFilePath,
  JSON.stringify(outputData, null, 2),
  'utf8',
);
console.log(`Output written to "${outputFilePath}"`);
console.log('Done!');
console.log('Performance benchmark completed.');

jcfilben and others added 2 commits April 15, 2025 09:01
Co-authored-by: Matthew Glissmann <mdglissmann@gmail.com>
@jcfilben jcfilben merged commit 9db1e5f into grommet:master Apr 15, 2025
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

React 19 upgrade
4 participants