lwc icon indicating copy to clipboard operation
lwc copied to clipboard

Synthetic shadow polyfill pollutes the global namespace

Open divmain opened this issue 2 years ago • 3 comments

Description

Loading the @lwc/synthetic-shadow polyfill pollutes the global namespace, resulting in errors in several web platform tests.

Steps to Reproduce

git clone https://github.com/divmain/wpt.git
cd wpt
git checkout synthetic-shadow
# Follow initial setup steps detailed in README.LWC.md
./wpt serve & open https://web-platform.test:8443/html/cross-origin-embedder-policy/shared-workers.https.html

Expected Results

Test passes.

Actual Results

Test never runs. @lwc/synthetic-shadow has already defined a function called create on window, which causes the attempted definition of create in shared-workers.https.html to fail.

Another example: createElement causes another test to fail.

Browsers Affected

Only Chromium-based browsers were tested. Test should also be validated as working against Firefox and Safari.

Version

  • LWC: 2.5.1

divmain avatar Sep 27 '21 08:09 divmain

This issue has been linked to a new work item: W-9950532

uip-robot-zz avatar Sep 27 '21 08:09 uip-robot-zz

I don't think this is a problem with @lwc/synthetic-shadow but rather how it's being loaded in the WPT repo. Technically @lwc/synthetic-shadow/dist/synthetic-shadow.js should be considered a module script rather than a classic script. It should not pollute the global namespace just by declaring top-level variables.

I'd say that the WPT script should be modified in these two places to wrap the script in an IIFE:

  • https://github.com/divmain/wpt/blob/2df40a6a7bbcc49ffe2d0a56664a9c11b45f58ac/package.json#L10
  • https://github.com/divmain/wpt/blob/synthetic-shadow/README.LWC.md#running-wpt-with-synthetic-shadow-polyfill

nolanlawson avatar Sep 27 '21 20:09 nolanlawson

I'm not sure what I was thinking with my comment above. @lwc/synthetic-shadow definitely modifies the global state; we do it all over the place here: https://github.com/salesforce/lwc/tree/master/packages/%40lwc/synthetic-shadow/src/polyfills

I don't think synthetic shadow could work if it didn't do this, though. It has to polyfill.

nolanlawson avatar Jan 10 '22 23:01 nolanlawson