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.

Sign up for GitHub

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

Jump to bottom

@azure/core-http isNode function will mistake electron for node #26003

Open
freedom-git opened this issue May 25, 2023 · 9 comments
Open

@azure/core-http isNode function will mistake electron for node #26003

freedom-git opened this issue May 25, 2023 · 9 comments
Assignees
@jeremymeng
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that

Comments

@freedom-git
Copy link

@azure/core-http@2.3.1"

For now it is :

const isNode = typeof process !== "undefined" &&
    !!process.version &&
    !!process.versions &&
    !!process.versions.node.

maybe this make more sense as electron is more like a browser than Node.

const isNode = typeof process !== "undefined" &&
    !!process.version &&
    !!process.versions &&
    !!process.versions.node&&
    !process.versions.electron;
@github-actions github-actions bot added customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-triage This is a new issue that needs to be triaged to the appropriate team. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels May 25, 2023
@xirzec xirzec added Client This issue points to a problem in the data-plane of the library. Azure.Core labels May 25, 2023
@xirzec
Copy link
Member

xirzec commented May 25, 2023

@jeremymeng can you look into this and also see what React-Native does?

/cc @mpodwysocki for Deno

@github-actions github-actions bot removed the needs-triage This is a new issue that needs to be triaged to the appropriate team. label May 25, 2023
@jeremymeng
Copy link
Contributor

@EmmaZhu suggested to add a browser version of isNode that just return false: #21110 (comment).

One interesting thing for Electron is that it is also kinda of Client and there's some option nodeIntegration that can enable node usage.

@jeremymeng
Copy link
Contributor

in Core v2 we do use a browser version of isNode from core-util. I will try porting it to core-http

@jeremymeng jeremymeng mentioned this issue May 25, 2023
Merged
@mpodwysocki mpodwysocki mentioned this issue May 26, 2023
Merged
3 tasks
jeremymeng added a commit that referenced this issue May 30, 2023
@jeremymeng
[core-http] export isNode from @azure/core-util (#26005)
97f4ad1 
core-http already depends on core-util. This PR removes core-http's
`isNode` and export from core-util instead.

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

### Issues associated with this PR
#26003
mpodwysocki added a commit that referenced this issue Jun 5, 2023
@mpodwysocki
[core-util] Add environment checks (#26018)
9b5a7a7 
### Packages impacted by this PR

- @azure/core-util

### Issues associated with this PR

#26003

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

Adds extra environment checks for browser or other environments such as
Deno, Bun, React-Native, Web Workers, etc

### 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
- [x] 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)
dgetu pushed a commit to dgetu/azure-sdk-for-js that referenced this issue Jun 5, 2023
@mpodwysocki @dgetu
[core-util] Add environment checks (Azure#26018)
09dcb00 
### Packages impacted by this PR

- @azure/core-util

### Issues associated with this PR

Azure#26003

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

Adds extra environment checks for browser or other environments such as
Deno, Bun, React-Native, Web Workers, etc

### 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
- [x] 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)
minhanh-phan pushed a commit to minhanh-phan/azure-sdk-for-js that referenced this issue Jun 12, 2023
@mpodwysocki @minhanh-phan
[core-util] Add environment checks (Azure#26018)
fe18e3b 
### Packages impacted by this PR

- @azure/core-util

### Issues associated with this PR

Azure#26003

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

Adds extra environment checks for browser or other environments such as
Deno, Bun, React-Native, Web Workers, etc

### 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
- [x] 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)
@xirzec
Copy link
Member

xirzec commented Jun 19, 2023

@mpodwysocki is this issue resolved in the latest core-util?

@amrit-moomie
Copy link

I have started seeing this issue again when running angular in electron

@xirzec
Copy link
Member

xirzec commented Apr 26, 2024

@amrit-moomie can you confirm what version of @azure dependencies you are using?

@amrit-moomie
Copy link

amrit-moomie commented Apr 26, 2024 •
edited
Loading

hi @xirzec

Actually I am using the latest release 12.17.0

I am running an angular project inside Electron shell (desktop app)

I started getting this issue recently #20095 (comment)

I believe isNode function is not returning properly here, that is the root cause

This was the original issue: #14742

@xirzec
Copy link
Member

xirzec commented May 2, 2024

@amrit-moomie if I understand you correctly, this is something you're experiencing with @azure/storage-blob and Electron? @mpodwysocki @jeremymeng

@amrit-moomie
Copy link

Yes.

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

@jeremymeng jeremymeng

Labels
Azure.Core Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@xirzec @amrit-moomie @jeremymeng @freedom-git