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

Add node dependencies and a browser field to services for the ws and node-fetch packages #5945

Merged
merged 2 commits into from Feb 5, 2019
Merged

Add node dependencies and a browser field to services for the ws and node-fetch packages #5945

merged 2 commits into from Feb 5, 2019

Conversation

jasongrout
Copy link
Contributor

Webpack should ignore our import of the ws and node-fetch nodejs packages because of the browser field (see https://github.com/defunctzombie/package-browser-field-spec#ignore-a-module).

Fixes #5856

@jasongrout
Add node dependencies and a browser field to services for the ws and …
a35219b 
…node-fetch packages

Webpack should ignore our import of the ws and node-fetch nodejs packages because of the browser field (see https://github.com/defunctzombie/package-browser-field-spec#ignore-a-module).

Fixes jupyterlab#5856
@jasongrout
integrity
433e0de
@jasongrout jasongrout added this to the 1.0 milestone Feb 5, 2019
@ian-r-rose
Copy link
Member

Do we have a way to check that this works as advertised?

@jasongrout
Copy link
Contributor Author

I manually checked it with a webpack build where I am installing the services package, i.e., the webpack build was failing before, but now is passing.

ian-r-rose
ian-r-rose approved these changes Feb 5, 2019
View reviewed changes
Copy link
Member

@ian-r-rose ian-r-rose left a comment

Choose a reason for hiding this comment

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

Okay, sounds good. Thanks @jasongrout

@jasongrout jasongrout merged commit 277da80 into jupyterlab:master Feb 5, 2019
@wolfv
Copy link
Member

wolfv commented Aug 2, 2019

Could it be possible that this breaks require('ws') in thirdparty libraries? I am currently having an issue like this with a thirdparty library (roslib.js) that uses ws to work in node and the browser, i suppose. However, inside JLab it compiles to something like window.ws = ws where ws is undefined, and then it fails to import the widget... is there some workaround? Right now I try to manually patch the offending library but it's a dependency of a dependency...

@jasongrout
Copy link
Contributor Author

Can you open a new issue about this, and link here? If there is something to do, the discussion should probably be in a new issue (especially just in case it is from something other than this PR)

@wolfv wolfv mentioned this pull request Aug 2, 2019
Closed
@wolfv
Copy link
Member

wolfv commented Aug 2, 2019

Hi @jasongrout I just opened this issue: #6934

@lock lock bot added the status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Sep 1, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Sep 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ian-r-rose ian-r-rose ian-r-rose approved these changes

Assignees
No one assigned
Labels
status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Projects
None yet
Milestone
1.0
Development

Successfully merging this pull request may close these issues.

Create build targets to output platform specific JS packages
3 participants
@jasongrout @ian-r-rose @wolfv