Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Select the parent of the script containing import.meta.document #15

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aaronmars
Copy link

See #14.
Aligns the transform for import.meta.document with the explainer/proposal at https://github.com/w3c/webcomponents/blob/gh-pages/proposals/html-modules-explainer.md

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@aaronmars
Copy link
Author

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@@ -51,7 +51,7 @@ export const htmlModuleToJsModuleMap =

if (property.type === 'Identifier') {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the problem here is that this check is not specific enough. The condition is checking for usages of import.meta.* as an expression, but isn't particular about import.meta.script (the original use case) vs import.meta.document.

We should probably change the condition to something like:

if (property.type === 'Identifier') {
  switch (property.name) {
    case 'script':
      // Do the current transform for script
      break;
    case 'document':
      // Do the new proposed transform for document
      break;
  }
}

@cdata
Copy link
Contributor

cdata commented May 7, 2019

Thanks for looking into this 🙌 left a note, LMKWYT!

@aaronmars
Copy link
Author

Oh yeah. Wow. I completely mis-read the code there. Makes sense now!
I'll get that fixed up.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants