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 for webpack: Export binary files in javascript package #3085

Merged
merged 2 commits into from
Sep 11, 2022

Conversation

dlech
Copy link
Contributor

@dlech dlech commented Sep 8, 2022

Description

This adds the distributed binary files to the "exports" section of the package.json file. This allows the files to be referenced, e.g. by require.resolve('pyodide/distutils.tar') which is useful for tools like webpack.

Checklists

This adds the distributed binary files to the "exports" section of
the package.json file. This allows the files to be referenced, e.g.
by `require.resolve('pyodide/distutils.tar')` which is useful for tools
like webpack.
@dlech
Copy link
Contributor Author

dlech commented Sep 8, 2022

Another user who would benefit from this change: https://github.com/mneil/pyodide-webpack/blob/main/patch.js

@hoodmane
Copy link
Member

hoodmane commented Sep 8, 2022

Well we can do this but we probably won't call it a breaking change if we rename, move, or delete these files since they are supposed to be internal.

@hoodmane
Copy link
Member

hoodmane commented Sep 8, 2022

But I suppose that the webpack use case only relies on the fact that we export all files that we need internally so if we move them it won't break webpack users.

@hoodmane
Copy link
Member

hoodmane commented Sep 8, 2022

Oh I see they are here:
https://github.com/mneil/pyodide-webpack/blob/main/webpack.config.js#L39-L44
We should probably have our own webpack plugin that does this so that downstream doesn't need to know the exact list of files we use.

@dlech
Copy link
Contributor Author

dlech commented Sep 8, 2022

We should probably have our own webpack plugin

That would be great - unless you use something like Create React App that doesn't allow access to the webpack config.

dlech added a commit to pybricks/pybricks-code that referenced this pull request Sep 8, 2022
This makes the required changes to use Pyodide and Python packages
offline and use a fixed version of the Python packages instead of
fetching the latest from PyPI.

There were a number of issues encountered with Pyodide webpack
compatibility that were fixed using `yarn patch`:
- pyodide/pyodide#3080
- pyodide/pyodide#3085
- pyodide/pyodide#3086
- pyodide/pyodide#3087

Issue: https://github.com/pybricks/pybricks-code/issues/932
@hoodmane
Copy link
Member

hoodmane commented Sep 8, 2022

unless you use something like Create React App that doesn't allow access to the webpack config.

I don't think Pyodide is ever likely to work with create react app, though of course if someone figures out a way maybe we can think about supporting it. The best we can do for now is to document that we don't know how to use them together.

@hoodmane
Copy link
Member

hoodmane commented Sep 8, 2022

webpack plugin

I would appreciate it if someone else could help with this though. I've tried to write a webpack plugin several times but always run into the problem that I was unable to use webpack with Pyodide at all. Perhaps with @mneil's example I would have an easier time, but the webpack plugin API is also very confusing. If it's just a wrapper around CopyPlugin then that probably isn't too hard.

In @mneil's example, he also patches pyodide.js to remove the global assignment to loadPyodide, but if you use pyodide.mjs instead of pyodide.js it doesn't assign loadPyodide to the global scope. We make pyodide.js out of pyodide.umd.ts which just adds the globalThis assignment to pyodide.ts:
https://github.com/pyodide/pyodide/blob/main/src/js/pyodide.umd.ts
Whereas pyodide.mjs is made from pyodide.ts directly. Though deleting that line is also an option if using webpack to load a module is not possible.

@hoodmane
Copy link
Member

hoodmane commented Sep 8, 2022

Thanks for all your help with this @dlech!

@dlech dlech mentioned this pull request Sep 9, 2022
@hoodmane
Copy link
Member

Okay after some discussion @mneil and @dlech have convinced me that this is a good idea. Thanks guys!

@hoodmane hoodmane changed the title export binary files in javascript package FIX for webpack: Export binary files in javascript package Sep 11, 2022
@hoodmane hoodmane merged commit 39522cd into pyodide:main Sep 11, 2022
hoodmane added a commit that referenced this pull request Sep 11, 2022
There is no longer a file called distutils.tar
@hoodmane
Copy link
Member

Since this PR was opened, we apparently changed the name of distutils.tar to distutils-1.0.0.zip.

@dlech dlech deleted the js-package-exports branch September 11, 2022 19:31
hoodmane pushed a commit to hoodmane/pyodide that referenced this pull request Sep 15, 2022
)

This adds the distributed binary files to the "exports" section of the package.json file.
This allows the files to be referenced, e.g. by `require.resolve('pyodide/distutils.tar')`
which is useful for tools like webpack.
hoodmane pushed a commit to hoodmane/pyodide that referenced this pull request Sep 15, 2022
)

This adds the distributed binary files to the "exports" section of the package.json file.
This allows the files to be referenced, e.g. by `require.resolve('pyodide/distutils.tar')`
which is useful for tools like webpack.
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.

2 participants