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

fix: resolve syntax error in old TypeScript versions #1166

Merged
merged 8 commits into from
Mar 25, 2024

Conversation

ahmedAlaaInstabug
Copy link
Contributor

@ahmedAlaaInstabug ahmedAlaaInstabug commented Mar 13, 2024

Description of the change

  1. fix unExpected token (??) on old node version

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Related issues

Fixes #1161
Jira ID = INSD-11129

Checklists

Development

  • Lint rules pass locally
  • The code changed/added as part of this pull request has been covered with tests

Code review

  • This pull request has a descriptive title and information useful to a reviewer
  • Issue from task tracker has a link to this pull request

@InstabugCI
Copy link
Collaborator

InstabugCI commented Mar 13, 2024

Coverage Report

Label Coverage Status
JavaScript 96.3%
Android 47.5%
iOS 41.7%

Generated by 🚫 dangerJS against 13de994

@a7medev a7medev changed the title fix: Unexpected token (??) on old node version fix: resolve syntax error in old TypeScript versions Mar 13, 2024
Copy link
Contributor

@a7medev a7medev left a comment

Choose a reason for hiding this comment

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

LGTM 👌🏼. Let's add a changelog item and we'd be good to go.

Let's also link it to a Jira card.

@InstabugCI
Copy link
Collaborator

Your snapshot has been generated! 🚀

Installation

You can install the snapshot through NPM:

npm install https://github.com/Instabug/Instabug-React-Native\#snapshot/fix-init-function-error-on-old-node-version

or Yarn:

yarn add https://github.com/Instabug/Instabug-React-Native\#snapshot/fix-init-function-error-on-old-node-version

@a7medev a7medev self-requested a review March 13, 2024 20:24
CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Ahmed Mahmoud <68241710+a7medev@users.noreply.github.com>
@@ -64,7 +64,9 @@ export const init = (config: InstabugConfig) => {
captureUnhandledRejections();

// Default networkInterceptionMode to JavaScript
config.networkInterceptionMode ??= NetworkInterceptionMode.javascript;
if (!config.networkInterceptionMode) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's change this if condition to explicitly check for null or undefined instead of !config.networkInterceptionMode since the enum values will get mapped to integer values with the first enum value being assigned the number 0 so if a user sets the networkInterceptionMode to the first enum value JavaScript will treat it as false thus changing the user's intended value to the default one even when it's actually set in the user's code.

In this case the first enum value is javascript which is the current default so it won't cause bugs, but if we changed the default to be something else users won't be able to set the interception mode to the first enum value as explained above.

Suggested change
if (!config.networkInterceptionMode) {
// The `== null` here acts as `=== null or === undefined`
if (config.networkInterceptionMode == null) {

Copy link
Contributor

@a7medev a7medev left a comment

Choose a reason for hiding this comment

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

LGTM 👌

@HeshamMegid HeshamMegid merged commit ee7f33d into master Mar 25, 2024
10 of 13 checks passed
@HeshamMegid HeshamMegid deleted the fix-init-function-error-on-old-node-version branch March 25, 2024 14:33
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.

Error loading JS bundle when running on Android
4 participants