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

Incomplete refactoring of function names #2107

Closed
jonahsnider opened this issue Oct 4, 2018 · 6 comments
Closed

Incomplete refactoring of function names #2107

jonahsnider opened this issue Oct 4, 2018 · 6 comments

Comments

@jonahsnider
Copy link

jonahsnider commented Oct 4, 2018

🐛 bug report

Parcel minimizes JavaScript in script tags by renaming functions, but doesn't change the original function names in fields like oninput.

🎛 Configuration

parcel build index.html

🤔 Expected Behavior

Minimize JavaScript and rename exampleFunction to a. Rename references to exampleFunction to a as well.

😯 Current Behavior

Just minimizes exampleFunction, not any references. This causes errors saying how that function doesn't exist.

🔦 Context

I defined a function in a script tag that I wanted to refer to later in oninput fields on HTML tags. Since this doesn't work, I can't minimize my JavaScript with Parcel.

💻 Code Sample

<!DOCTYPE html>
<html>
<head>
  <script>
    function exampleFunction(string) {
      console.log(string.toUpperCase());
    }  
  </script>
</head>
<body>
  <input type="text" oninput="exampleFunction(this.value)">
</body>
</html>

Open up the web inspector console and type some characters in the text box.

Now do the same with the result of parcel build index.html

Here's the compressed code from parcel (I beautified it):

<!DOCTYPE html>
<html>

<head>
  <script>function o(o){console.log(o.toUpperCase())}</script>
</head>

<body> <input type="text" oninput="exampleFunction(this.value)"> </body>

</html>

As you can see, the oninput field was left unchanged, causing it to error.

🌍 Your Environment

Software Version
Parcel 1.10.1
@9oelM
Copy link

9oelM commented Oct 15, 2018

I just briefly scanned over the source of parcel and I think it does not process inline javascript expressions put inside tags like <a> or <input> inside an html asset. It seems only to look for script tags inside html assets. Maybe I will have a deeper look when I have some time.

However I found a temporary workaround, which is to just explicitly attach it to window (or any other global, according to your environment):

Workaround code

<!DOCTYPE html>
<html>
<head>
  <script>
    window.exampleFunction = function exampleFunction(string) {
      console.log(string.toUpperCase());
    }  
  </script>
</head>
<body>
  <input type="text" onInput="exampleFunction(this.value)">
</body>
</html>

Transformed code (I beautified it because parcel build minifies)

<!DOCTYPE html>
<html>
    <head>
        <script>
            window.exampleFunction = function(o) {
                console.log(o.toUpperCase())
            }
            ;
        </script>
    </head>
    <body>
        <input type="text" oninput="exampleFunction(this.value)">
    </body>
</html>

Now it works!
Hope the workaround helps you.

@github-actions github-actions bot added the Stale Inactive issues label Jan 17, 2020
@jonahsnider
Copy link
Author

Issue is still present in Parcel v1.12.4

@mischnic mischnic removed the Stale Inactive issues label Jan 17, 2020
@parcel-bundler parcel-bundler deleted a comment from github-actions bot Jan 17, 2020
@github-actions github-actions bot added the Stale Inactive issues label Jul 16, 2020
@jonahsnider
Copy link
Author

Issue is still present.

@github-actions github-actions bot removed the Stale Inactive issues label Jul 16, 2020
@DeMoorJasper
Copy link
Member

@pizzafox I assume this has actually been fixed in Parcel 2 as we now process inline scripts properly

@jonahsnider
Copy link
Author

Issue is still present in Parcel v2.0.0-nightly.339+1f558665.

The output has now changed to the following (formatted):

<!DOCTYPE html>
<html>
  <head>
    <script>
      //# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJtYXBwaW5ncyI6IiIsInNvdXJjZXMiOltdLCJzb3VyY2VzQ29udGVudCI6W10sIm5hbWVzIjpbXSwidmVyc2lvbiI6MywiZmlsZSI6ImluZGV4LkhBU0hfUkVGXzA1ZmIzNmVkNWI2YTU1NTFmNWQ2OTk1YmU4OTI2YzE3LmpzLm1hcCJ9
    </script>
  </head>
  <body>
    <input type="text" oninput="exampleFunction(this.value)" />
  </body>
</html>

@devongovett
Copy link
Member

Appears to be fixed in the latest nightlies (as of #6247). We now treat script tags without type="module" how the browser does, i.e. top-level variables are globals and won't be renamed or wrapped in a function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants