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

Add Next.js example to check bundle size on web #3650

Closed
wants to merge 23 commits into from

Conversation

tomekzaw
Copy link
Member

@tomekzaw tomekzaw commented Oct 6, 2022

Description

This PR adds NextExample app (based on https://github.com/nandorojo/reanimated-tree-shaking from @nandorojo) which can be used to check Reanimated bundle size as well as its tree-shaking capabilities when used on web (specifically, in Next.js project).

The motivation behind this PR is a significant increase (~10x) of bundle size on web when using Reanimated 3.0.0-rc.3 compared to 3.0.0-rc.2 caused by Babel plugin appending worklets with sourcemaps as comments for debugging worklet runtime with Flipper which is not necessary on web.

Reanimated version only Animated.View with sample animation*
3.0.0-rc.2 17.3 kB 27.5 kB
3.0.0-rc.3 126 kB ⚠️ 224 kB ⚠️
this PR 17.3 kB ✅ 27.2 kB ✅

* - Animated.View, useSharedValue, useAnimatedStyle, withTiming, withRepeat

TODO:

  • properly disable generating sourcemaps on web
  • confirm that sourcemaps are not necessary in dev mode
  • "react-native-reanimated": "link:../" instead of 3.0.0-rc.3 to make local development easier
  • check if fast refresh in core.ts works properly
  • use SWC plugin
  • show package size as CI output

@tomekzaw tomekzaw changed the title Add Next.js example to check bundle size Add Next.js example to check bundle size on web Oct 6, 2022
@tomekzaw tomekzaw marked this pull request as ready for review October 10, 2022 13:27
@tomekzaw tomekzaw requested a review from piaskowyk October 10, 2022 13:27
"raf": "^3.4.1",
"react": "^18.2.0",
"react-dom": "^18.2.0",
"react-native-reanimated": "3.0.0-rc.3",
Copy link
Member Author

Choose a reason for hiding this comment

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

Is there any option to use link:../ here so we can have fast refresh when we edit package files? cc @nandorojo

Copy link
Contributor

Choose a reason for hiding this comment

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

an easier method is probably to un-comment the webpack config resolver in next.config.js and point it to the right place. but link might also work. i assume this is using yarn link? i don’t think i’ve tried this before

Copy link
Member Author

Choose a reason for hiding this comment

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

Does this approach support fast-refresh also when changing Reanimated codebase, e.g. src/reanimated2/core.ts?

- main

env:
MAX_SIZE_KB: 30
Copy link
Member Author

Choose a reason for hiding this comment

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

The plan is that we will increase this value accordingly if needed.

plugin.js Outdated
@@ -282,11 +282,19 @@ function isRelease() {
return process.env.BABEL_ENV === 'release';
}

function isProduction() {
return process.env.NODE_ENV === 'production'; // for web
Copy link
Member Author

@tomekzaw tomekzaw Oct 10, 2022

Choose a reason for hiding this comment

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

Is there any better way to detect production build for web? cc @nandorojo

Copy link
Contributor

Choose a reason for hiding this comment

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

this is correct

Copy link
Contributor

Choose a reason for hiding this comment

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

that said, should these source maps still get generated for web, even in dev mode?

Copy link
Member Author

@tomekzaw tomekzaw Nov 21, 2022

Choose a reason for hiding this comment

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

Indeed, they don't need to be generated even in dev mode.

Here's a screenshot of debugger stopping on a breakpoint in a worklet without sourcemaps (there's no sourceMappingURL in _f.asString):

Q: Is there any way to check if Babel transpiles for web or for React Native?

NextExample/app.json Outdated Show resolved Hide resolved
@@ -0,0 +1,9 @@
module.exports = {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should use the reanimated SWC plug-in instead of Babel? Or perhaps do a run with and a run without it, using an env variable?

For reference: https://github.com/showtime-xyz/showtime-frontend/blob/staging/apps/next/next.config.js#L91-L99

This isn’t a requirement for this PR, but would be useful for testing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea, we can test the SWC plugin as well once it's merged (in a separate PR).

ignoreDuringBuilds: true,
},
webpack(config, _options) {
// config.resolve.alias = {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you could make this point to the locally-built folder.

@nandorojo
Copy link
Contributor

Hey, just checking on the status of this PR :)

@tomekzaw tomekzaw marked this pull request as draft November 29, 2022 13:10
@piaskowyk
Copy link
Member

@bartlomiejbloniarz works on the new version of this test.

@piaskowyk piaskowyk closed this Oct 2, 2023
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.

3 participants