MetaMask / MetaMask/metamask-mobile
Dev Experience - Modernize Polyfill
- Dominant language
- TypeScript
- Stars
- 3k
- Forks
- 1.7k
- Avg merge
- 1d 14h
- Merged PRs (30d)
- 669
Description
### What is this about?
As a developer, I need to modernize the polyfill approach for React Native in MetaMask Mobile to replace the problematic rn-nodeify implementation with a more maintainable Metro resolver configuration.
### Scenario
- GIVEN a developer needs to modify provider code
- WHEN they make changes to the inpage bridge
- THEN they should be able to test those changes without running the full setup process again
- AND the polyfill system should not directly modify node_modules files
### Design
N/A - This is a technical infrastructure improvement without UI changes.
### Technical Details
#### Current Issues
1. **rn-nodeify Problems**
- Directly modifies files in node_modules
- Requires re-running setup for provider changes
- Complicates debugging and development workflow
- Creates potential conflicts with the patch system
2. **Provider Injection Issues**
- Current flow: `app/components/Views/BrowserTab/BrowserTab.tsx —injects—> app/core/InpageBridgeWeb3.js <—built by— scripts/inpage-bridge/webpack.config.js`
- After `yarn setup` runs `rn-nodeify`, attempting to manually rebuild the provider fails with multiple dependency errors:
- `Can't resolve 'util-deprecate'` across nested dependencies
- React Native import errors with `typeof` keyword
- Stream polyfill inconsistencies between webpack and rn-nodeify
#### Modern Approach: Metro Resolver Configuration
1. **Replace rn-nodeify with metro.config.js configuration**
```javascript
// metro.config.js
const { getDefaultConfig } = require('@react-native/metro-config');
const path = require('path');
module.exports = (async () => {
const config = await getDefaultConfig(__dirname);
// Add polyfill mappings
config.resolver.extraNodeModules = {
'crypto': path.resolve(__dirname, 'node_modules/react-native-crypto'),
'stream': path.resolve(__dirname, 'node_modules/stream-browserify'),
'http': path.resolve(__dirname, 'node_modules/@tradle/react-native-http'),
'https': path.resolve(__dirname, 'node_modules/https-browserify'),
'vm': path.resolve(__dirname, 'node_modules/vm-browserify'),
'os': path.resolve(__dirname, 'node_modules/react-native-os'),
'net': path.resolve(__dirname, 'node_modules/react-native-tcp'),
'fs': path.resolve(__dirname, 'node_modules/react-native-level-fs'),
'util-deprecate': path.resolve(__dirname, 'node_modules/util-deprecate'),
// Add all other polyfills from package.json
};
return config;
})();
```
2. **Update webpack.config.js for the inpage bridge**
```javascript
// scripts/inpage-bridge/webpack.config.js
const webpack = require('webpack');
const path = require('path');
const { readFileSync } = require('fs');
// ...existing code...
const config = {
// ...existing configuration...
resolve: {
fallback: {
buffer: require.resolve('buffer'),
stream: require.resolve('stream-browserify'),
_stream_transform: require.resolve('readable-stream/transform'),
_stream_readable: require.resolve('readable-stream/readable'),
_stream_writable: require.resolve('readable-stream/writable'),
_stream_duplex: require.resolve('readable-stream/duplex'),
_stream_passthrough: require.resolve('readable-stream/passthrough'),
'util-deprecate': require.resolve('util-deprecate'),
// Add other missing polyfills
},
alias: {
// Avoid React Native imports in webpack
'react-native': path.resolve(__dirname, './polyfills/react-native-stub.js'),
}
},
// ...rest of config...
};
```
3. **Create Development Mode for Provider**
```javascript
// scripts/dev-inpage-bridge.js
const webpack = require('webpack');
const path = require('path');
const config = require('./inpage-bridge/webpack.config');
const fs = require('fs');
// Set to development mode
config.mode = 'development';
config.watch = true;
config.watchOptions = {
ignored: /node_modules/,
};
// Create webpack compiler
const compiler = webpack(config);
compiler.watch({}, (err, stats) => {
if (err) {
console.error(err);
return;
}
// Copy the output to the app/core directory
fs.copyFileSync(
path.resolve(__dirname, './inpage-bridge/dist/index.js'),
path.resolve(__dirname, '../app/core/InpageBridgeWeb3.js')
);
console.log('InpageBridge rebuilt and copied. Refresh your app to see changes.');
});
```
4. **React Native stubs for webpack**
```javascript
// scripts/inpage-bridge/polyfills/react-native-stub.js
module.exports = {
Platform: {
OS: 'web',
select: (obj) => obj.web || obj.default || {},
},
NativeModules: {},
// Add other commonly used React Native APIs as needed
};
```
5. **Package.json scripts**
```json
{
"scripts": {
"watch:inpage": "node scripts/dev-inpage-bridge.js",
"build:inpage": "./scripts/build-inpage-bridge.sh"
}
}
```
### Threat Modeling Framework
- **What are we working on?** Replacing rn-nodeify with Metro resolver configuration to improve development workflow and maintainability.
- **What can go wrong?**
- Missing polyfill mappings could cause runtime errors
- Inconsistency between development and production builds
- Breaking existing functionality that depends on the current system
- **What are we going to do about it?**
- Document all current polyfill mappings before migration
- Create comprehensive tests for polyfill functionality
- Implement gradual migration with feature flagging if necessary
- **Did we do a good job?** Success criteria include faster development cycles, no direct node_modules modifications, and all existing functionality working correctly.
### Acceptance Criteria
- All Node.js modules are properly polyfilled with Metro resolver
- No direct modification of node_modules files
- Provider changes can be tested without re-running full setup
- All existing functionality continues to work correctly
- Development workflow is simplified with watch mode for inpage bridge
- Build process is updated to use the new approach
- Documentation is updated to reflect the new system
- Migration steps are clear and well-tested
### Stakeholder Review
- [x] Engineering (needed in most cases)
- [ ] Design
- [ ] Product
- [x] QA (automation tests are required to pass before merging PRs but not all changes are covered by automation tests - please review if QA is needed beyond automation tests)
- [ ] Security
- [ ] Legal
- [ ] Marketing
- [ ] Management (please specify)
- [ ] Other (please specify)
### References
- Current implementation in package.json and setup.mjs
- Metro bundler documentation: https://facebook.github.io/metro/docs/configuration/
- React Native Node.js compatibility layer documentation
Contributor guide
Research direction
Start by reading package.json and setup.mjs, then inspect app/components/Views/BrowserTab/BrowserTab.tsx, app/core/InpageBridgeWeb3.js, and scripts/inpage-bridge/webpack.config.js to map the current rn-nodeify flow. Review the proposed Metro resolver, development watch mode, and webpack polyfills against the stated acceptance criteria. Done means provider changes work without rerunning setup, node_modules is not modified, existing functionality remains intact, and the build and documentation are updated.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, node.js, react-native, typescript, webpack
- Domain
- build-system, developer-experience, mobile, tooling
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 28/100