-
Notifications
You must be signed in to change notification settings - Fork 0
Modernize dependencies to Babel 7 and Gulp 5, add comprehensive tests #1
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
Conversation
Co-authored-by: joshuabalduff <11274822+joshuabalduff@users.noreply.github.com>
Co-authored-by: joshuabalduff <11274822+joshuabalduff@users.noreply.github.com>
Co-authored-by: joshuabalduff <11274822+joshuabalduff@users.noreply.github.com>
|
@copilot can you add tests? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR modernizes the react-native-hr package by upgrading from Babel 6 to Babel 7, Gulp 3 to Gulp 5, and adding explicit peer dependencies for React and React Native. The goal is to ensure the package works with modern React Native versions and resolves security vulnerabilities from outdated dependencies.
Key Changes:
- Upgraded build tooling from Babel 6 to Babel 7 and Gulp 3 to Gulp 5
- Added peer dependencies for React (>=16.8.0) and React Native (>=0.60.0)
- Updated configuration files (.babelrc) and build scripts to use new Babel preset naming
Reviewed changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Updated dependencies to Babel 7 (@babel/cli, @babel/core, @babel/preset-env, @babel/preset-react), Gulp 5, and added React/React Native peer dependencies |
| .babelrc | Created new Babel 7 configuration with @babel/preset-env and @babel/preset-react presets |
| gulpfile.js | Modernized Gulp tasks to use Gulp 5's named function exports pattern and removed inline preset configuration |
| dist/index.js | Generated output file transpiled with Babel 7 (auto-generated, no review needed) |
| examples/hr.dist.js | Generated output file transpiled with Babel 7 (auto-generated, no review needed) |
| README.md | Added compatibility section documenting React >= 16.8.0 and React Native >= 0.60.0 requirements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
.babelrc
Outdated
| ["@babel/preset-env", { | ||
| "targets": { | ||
| "node": "current" | ||
| } | ||
| }], |
Copilot
AI
Dec 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The .babelrc configuration targets "node": "current" but this package is meant to be consumed by React Native applications. React Native uses a JavaScript runtime that's not Node.js. This target configuration could result in transpiled code that's not optimized for React Native environments.
Consider removing the targets option to use Babel's default targets, or specify React Native-specific targets if needed.
| ["@babel/preset-env", { | |
| "targets": { | |
| "node": "current" | |
| } | |
| }], | |
| ["@babel/preset-env", {}], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. Removed the Node.js target configuration to use Babel's default targets which are appropriate for React Native environments. (d83ccf4)
package.json
Outdated
| "license": "ISC", | ||
| "peerDependencies": { | ||
| "react": ">=16.8.0", | ||
| "react-native": ">=0.60.0" |
Copilot
AI
Dec 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The peer dependencies should also include prop-types since the component uses it extensively. Without listing prop-types as a peer dependency, users may encounter runtime errors if they don't have it installed in their projects.
Add:
"prop-types": ">=15.0.0"| "react-native": ">=0.60.0" | |
| "react-native": ">=0.60.0", | |
| "prop-types": ">=15.0.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. Added prop-types (>=15.0.0) to peer dependencies. (d83ccf4)
package.json
Outdated
| "build": "babel src/index.js --out-file examples/hr.dist.js --presets @babel/preset-env,@babel/preset-react", | ||
| "start": "gulp", | ||
| "watch": "babel src --out-dir dist --watch --presets es2015,react" | ||
| "watch": "babel src --out-dir dist --watch --presets @babel/preset-env,@babel/preset-react" |
Copilot
AI
Dec 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The build and watch scripts pass presets directly via the command line, but these presets are also configured in .babelrc. This creates duplicate configuration that could lead to maintenance issues if the presets need to be updated in the future.
Consider removing the --presets flags from the CLI commands and relying solely on .babelrc for consistency:
"build": "babel src/index.js --out-file examples/hr.dist.js",
"watch": "babel src --out-dir dist --watch"There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. Removed --presets flags from build and watch scripts. Configuration now centralized in .babelrc file. (d83ccf4)
examples/hr.dist.js
Outdated
| function _assertThisInitialized(e) { if (void 0 === e) throw new ReferenceError("this hasn't been initialised - super() hasn't been called"); return e; } | ||
| function _isNativeReflectConstruct() { try { var t = !Boolean.prototype.valueOf.call(Reflect.construct(Boolean, [], function () {})); } catch (t) {} return (_isNativeReflectConstruct = function _isNativeReflectConstruct() { return !!t; })(); } | ||
| function _getPrototypeOf(t) { return _getPrototypeOf = Object.setPrototypeOf ? Object.getPrototypeOf.bind() : function (t) { return t.__proto__ || Object.getPrototypeOf(t); }, _getPrototypeOf(t); } | ||
| function _inherits(t, e) { if ("function" != typeof e && null !== e) throw new TypeError("Super expression must either be null or a function"); t.prototype = Object.create(e && e.prototype, { constructor: { value: t, writable: !0, configurable: !0 } }), Object.defineProperty(t, "prototype", { writable: !1 }), e && _setPrototypeOf(t, e); } |
Copilot
AI
Dec 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This write to property 'prototype' is useless, since another property write always overrides it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed by adding @babel/plugin-transform-runtime. The generated code now uses external helpers from @babel/runtime instead of inline helpers, which eliminates the redundant prototype writes and reduces code size. (e9365cf)
- Remove Node.js target from .babelrc (use default for React Native) - Add prop-types to peer dependencies - Remove duplicate preset flags from build scripts (rely on .babelrc) - Add @babel/preset-flow for React Native compatibility - Add Jest test framework with 8 comprehensive tests - Configure Jest without react-native preset to avoid Flow parsing issues - All tests passing (8/8) Co-authored-by: joshuabalduff <11274822+joshuabalduff@users.noreply.github.com>
- Explicitly add babel-jest transformer for Jest tests Co-authored-by: joshuabalduff <11274822+joshuabalduff@users.noreply.github.com>
Co-authored-by: joshuabalduff <11274822+joshuabalduff@users.noreply.github.com>
|
@copilot apply changes based on the comments in this thread |
- Add @babel/plugin-transform-runtime and @babel/runtime - Use external helpers instead of inline helpers - Fixes redundant prototype writes in generated code - Reduces code size and improves maintainability - All tests still passing (8/8) Co-authored-by: joshuabalduff <11274822+joshuabalduff@users.noreply.github.com>
All changes from that review thread have already been applied:
All tests passing (8/8), build working, 0 vulnerabilities. |
Plan to Update react-native-hr
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.