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

[Core] react-native issues/regressions #30065

Open
jeremymeng opened this issue Jun 14, 2024 · 3 comments
Open

[Core] react-native issues/regressions #30065

jeremymeng opened this issue Jun 14, 2024 · 3 comments
Assignees
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library.

Comments

@jeremymeng
Copy link
Contributor

jeremymeng commented Jun 14, 2024

With this option from React-Native, our react-native conditional exports are respected, but issues also surface. Previously most of these were mapped to their browser version for react-native.

  • core-rest-pipeline/src/util/inspect.ts is importing from "node:util", need a react-native version (same as browser should be good)
  • core-rest-pipeline/src/util/concat.ts needs a react-native version (same as browser should be good too)
  • core-rest-pipeline/src/util/userAgentPlatform.ts the dynamic import await import causing react-native error compiling JS code because it puts await in an non-async context.
    • could we do `import {...} from "react-native" but without adding a dependency on "react-native"? maybe just need to add a typing/declared module for it
  • node js only policies also need react-native version: proxyPolicy. We could do no-op, instead of throwing like in browser version
  • logger/src/log.ts is import from node:` modules, we probably could make a react-native version using same code from browser version.
    import { EOL } from "node:os";
  • core-util/src/sha256.ts is importing from crypto
    • we could use a copy of browser version, which would require customers to provide polyfills for the crypto API that we use, or
    • we could implement react-native version using the platform API if possible. I only find that Expo provides some API but extra steps are needed for bare react-native projects https://docs.expo.dev/versions/latest/sdk/crypto/
    • is there a polyfill package that would work with import { createHash, createHmac } from "crypto"; with some resolver configuration?
  • core-util/src/byteEncoding.ts is using Buffer, browser version should work too for react-native.

core-amqp has similar problems too: util/

  • checkNetworkConnection
  • hmacSha256
  • runtimeInfo
@jeremymeng jeremymeng added Client This issue points to a problem in the data-plane of the library. Azure.Core labels Jun 14, 2024
@jeremymeng
Copy link
Contributor Author

originated from #29980

mpodwysocki added a commit that referenced this issue Jun 17, 2024
### Packages impacted by this PR

- @azure/logger

### Issues associated with this PR

- #30065

### Describe the problem that is addressed by this PR

Adds logging support through the browser implementation of the logger
logic.

### What are the possible designs available to address the problem? If
there are more than one possible design, why was the one in this PR
chosen?


### Are there test cases added in this PR? _(If not, why?)_


### Provide a list of related PRs _(if any)_


### Command used to generate this PR:**_(Applicable only to SDK release
request PRs)_

### Checklists
- [ ] Added impacted package name to the issue description
- [ ] Does this PR needs any fixes in the SDK Generator?** _(If so,
create an Issue in the
[Autorest/typescript](https://github.com/Azure/autorest.typescript)
repository and link it here)_
- [ ] Added a changelog (if necessary)
mpodwysocki added a commit that referenced this issue Jun 18, 2024
### Packages impacted by this PR

- @azure/core-rest-pipeline
- @typespec/ts-http-runtime

### Issues associated with this PR

- #30065

### Describe the problem that is addressed by this PR

Adds web crypto APIs for React-Native calls. Requires the consumer to
polyfill the Web Crypto APIs in order for these modules to work.

### What are the possible designs available to address the problem? If
there are more than one possible design, why was the one in this PR
chosen?


### Are there test cases added in this PR? _(If not, why?)_


### Provide a list of related PRs _(if any)_


### Command used to generate this PR:**_(Applicable only to SDK release
request PRs)_

### Checklists
- [ ] Added impacted package name to the issue description
- [ ] Does this PR needs any fixes in the SDK Generator?** _(If so,
create an Issue in the
[Autorest/typescript](https://github.com/Azure/autorest.typescript)
repository and link it here)_
- [ ] Added a changelog (if necessary)
mpodwysocki added a commit that referenced this issue Jun 18, 2024
### Packages impacted by this PR

- @azure/core-rest-pipeline
- @typespec/ts-http-runtime

### Issues associated with this PR

- #30065

### Describe the problem that is addressed by this PR

Adds more support for user agents for browser and fixes React-Native
implementation to import "react-native".

### What are the possible designs available to address the problem? If
there are more than one possible design, why was the one in this PR
chosen?


### Are there test cases added in this PR? _(If not, why?)_


### Provide a list of related PRs _(if any)_


### Command used to generate this PR:**_(Applicable only to SDK release
request PRs)_

### Checklists
- [ ] Added impacted package name to the issue description
- [ ] Does this PR needs any fixes in the SDK Generator?** _(If so,
create an Issue in the
[Autorest/typescript](https://github.com/Azure/autorest.typescript)
repository and link it here)_
- [ ] Added a changelog (if necessary)
mpodwysocki added a commit that referenced this issue Jun 18, 2024
### Packages impacted by this PR

- @azure/core-rest-pipeline
- @typespec/ts-http-runtime

### Issues associated with this PR

- #30065

### Describe the problem that is addressed by this PR

Updates the following:
- Adds Proxy Policy for react-native to throw since it is not supported,
same for browser.
 - Adds React-Native support for concat streams same as browser support
 - Adds React-Native support for inspect same as browser support

### What are the possible designs available to address the problem? If
there are more than one possible design, why was the one in this PR
chosen?


### Are there test cases added in this PR? _(If not, why?)_


### Provide a list of related PRs _(if any)_


### Command used to generate this PR:**_(Applicable only to SDK release
request PRs)_

### Checklists
- [ ] Added impacted package name to the issue description
- [ ] Does this PR needs any fixes in the SDK Generator?** _(If so,
create an Issue in the
[Autorest/typescript](https://github.com/Azure/autorest.typescript)
repository and link it here)_
- [ ] Added a changelog (if necessary)
@jeremymeng jeremymeng self-assigned this Jun 18, 2024
@tonylucgi
Copy link

Can you tell what the expected date is when this fixing gets into the release ?

@jeremymeng
Copy link
Contributor Author

@tonylucgi the planned date for the July release is 7/11/2024

jeremymeng added a commit that referenced this issue Jul 12, 2024
similar to PR #30080

-------

### Packages impacted by this PR
`@azure/core-amqp`

### Issues associated with this PR

#30065
jeremymeng added a commit that referenced this issue Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library.
Projects
None yet
Development

No branches or pull requests

2 participants