-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
MediaUtils
: move window
access to function bodies
#46970
Conversation
Size Change: +44 B (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
Flaky tests detected in 9fc28ef. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/3872999115
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks for working on this 👍
What?
Move access to the global
window.wp
object to respective function bodies for the@wordpress/media-utils
and@wordpress/blob
packages.This issue was brought up in the WordPress Slack core-js channel (https://wordpress.slack.com/archives/C5UNMSU4R/p1668693333126039).
Why?
Currently, it's not possible to use those packages (or packages that depend on them) in a Node.js environment because the global
window
is not available there.How?
By moving access to
window.wp
to those functions a Node.js process won't fail during static initialization.Testing Instructions
To test whether the import of the
@wordpress/media-utils
package fails create a local project and add the@wordpress/media-utils
package as a dependency. It's sufficient to just imort that package in a file. I used these steps:Create a file
index.mjs
which contains the corresponding importThen execute
node index.mjs
on the command line.To test whether this patch fixes the process failure described above use that exact project and link the dependency by using
npm link
.packages/media-utils
npm link
(make sure to runnpm build:packages
before)npm link @wordpress/media-utils
(it may be necessary to do that for@wordpress/blob
, too).node index.mjs
again