Skip to content

Implement asset hashing and child window handling #14

Open
RajatRaiD11 wants to merge 2 commits into
mainfrom
test-other
Open

Implement asset hashing and child window handling #14
RajatRaiD11 wants to merge 2 commits into
mainfrom
test-other

Conversation

@RajatRaiD11

@RajatRaiD11 RajatRaiD11 commented Mar 30, 2025

Copy link
Copy Markdown
Owner

PR Type

Enhancement


Description

  • Added a new script to generate a hash of all the React Native bundled assets.

  • Introduced a new C++ file for child window handling.


Changes walkthrough 📝

Relevant files
Enhancement
generateBundledResourcesHash.js
Implement asset hashing in React Native                                   

generateBundledResourcesHash.js

  • Created a new JavaScript file that generates a hash of all the React
    Native bundled assets.
  • The script computes the hash for each file to generate a manifest, and
    then computes a hash over the entire manifest to generate the final
    hash.
  • The final hash is saved to the APK's assets directory.
  • +125/-0 
    cropnlock.cpp
    Implement child window handling in C++                                     

    cropnlock.cpp

  • Introduced a new C++ file for handling child windows.
  • The file includes methods for registering the window class, creating
    the child window, and handling window messages.
  • +56/-0   

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @RajatRaiD11

    Copy link
    Copy Markdown
    Owner Author

    PR Description updated to latest commit (e776ab4)

    @RajatRaiD11

    RajatRaiD11 commented Mar 30, 2025

    Copy link
    Copy Markdown
    Owner Author

    PR Reviewer Guide 🔍

    (Review updated until commit e776ab4)

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Code Quality

    The script uses synchronous file operations which can block the Node.js event loop and potentially impact performance. Consider refactoring to use asynchronous file operations.

    fs.writeFileSync(savedResourcesManifestPath, finalHash);
    
    // "CodePushHash.json" file name breaks flow type checking.
    // To fix the issue we need to delete "CodePushHash.json" file and
    // use "CodePushHash" file name instead to store the hash value.
    // Relates to https://github.com/microsoft/react-native-code-push/issues/577
    var oldSavedResourcesManifestPath = assetsDir + "/" + CODE_PUSH_HASH_OLD_FILE_NAME;
    if (fs.existsSync(oldSavedResourcesManifestPath)) {
        fs.unlinkSync(oldSavedResourcesManifestPath);
    Code Quality

    The ChildWindow class does not seem to handle any user input or system events other than WM_DESTROY. If there are other events that need to be handled, they should be implemented.

    LRESULT ChildWindow::MessageHandler(UINT const message, WPARAM const wparam, LPARAM const lparam)
    {
        switch (message)
        {
        case WM_DESTROY:
            break;
        default:
            return base_type::MessageHandler(message, wparam, lparam);
        }
        return 0;
    }

    @RajatRaiD11 RajatRaiD11 added the enhancement New feature or request label May 11, 2025
    @RajatRaiD11 RajatRaiD11 changed the title Test other Implement asset hashing and child window handling May 11, 2025
    @RajatRaiD11

    Copy link
    Copy Markdown
    Owner Author

    PR Description updated to latest commit (e776ab4)

    @RajatRaiD11

    Copy link
    Copy Markdown
    Owner Author

    Persistent review updated to latest commit e776ab4

    @RajatRaiD11

    Copy link
    Copy Markdown
    Owner Author

    PR Review Summary

    This pull request introduces two new files:

    1. generateBundledResourcesHash.js - A script to generate a hash of all React Native bundled assets for optimizing update downloads.
    2. cropnlock.cpp - A C++ file for handling child windows, including methods for window class registration, creation, and message handling.

    🟢 Strengths

    • The asset hashing script helps prevent downloading identical updates already present in the binary, optimizing the update process.
    • The child window handling functionality in C++ provides a reusable component for managing child windows.
    • Both files include clear comments explaining their purpose and implementation details.

    🟠 Recommendations

    • Line 41 in generateBundledResourcesHash.js: Consider using a more secure hash algorithm like SHA-256 or SHA-3 instead of SHA-1, which is now considered insecure.
    • Line 20 in cropnlock.cpp: The ClassName constant could be made more descriptive or follow a naming convention for better readability.
    • Line 44 in cropnlock.cpp: The CreateWindowExW call could benefit from additional error handling or validation of input parameters.

    🔴 Critical Issues

    • None identified.

    📊 Test Coverage

    The pull request does not include any test cases or information about test coverage for the added functionality.

    📝 Documentation

    Both files include inline comments explaining their purpose and implementation details, which serves as documentation. However, additional documentation or examples for integrating and using these components may be beneficial.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Labels

    enhancement New feature or request Review effort 3/5

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    1 participant