Skip to content

fix: a missing semicolon generated by the publicPathRuntime could lead to concatenation issues#15003

Closed
toovy wants to merge 3 commits intowebpack:mainfrom
triggercode:main
Closed

fix: a missing semicolon generated by the publicPathRuntime could lead to concatenation issues#15003
toovy wants to merge 3 commits intowebpack:mainfrom
triggercode:main

Conversation

@toovy
Copy link
Copy Markdown

@toovy toovy commented Dec 16, 2021

Motivation

In JS you can opt out on using ; on line endings. Some consider this good style, some bad style. I don't care until it generates issues. When concatenating source code missing ; can lead to issues. Consider the generated code:

generated-source

In my case the generated code that does not use a ; led to issues when using it with a non-webpack concatenation (WPRocket):

inline-source

Obviously most of the times you would just minify using webpack with the terser plugin, which works flawlessly, but in our case we will use WPRockets concatenation and minification and want the unminified code.

What kind of change does this PR introduce?

Bugfix

Did you add tests for your changes?

Yes

Does this PR introduce a breaking change?

No

What needs to be documented once your changes are merged?

Nothing

@linux-foundation-easycla
Copy link
Copy Markdown

CLA Not Signed

@webpack-bot
Copy link
Copy Markdown
Contributor

For maintainers only:

  • This needs to be documented (issue in webpack/webpack.js.org will be filed when merged)
  • This needs to be backported to webpack 4 (issue will be created when merged)

@alexander-akait
Copy link
Copy Markdown
Member

We can fix it, but I recommend report the problem in WPRockets, because bundled packages can have the same code

@linux-foundation-easycla
Copy link
Copy Markdown

CLA Not Signed

@alexander-akait
Copy link
Copy Markdown
Member

@toovy Can you accept CLA?

@sokra
Copy link
Copy Markdown
Member

sokra commented Dec 17, 2021

and update the snapshot: yarn jest Stats -u

@linux-foundation-easycla
Copy link
Copy Markdown

CLA Not Signed

@linux-foundation-easycla
Copy link
Copy Markdown

CLA Not Signed

@toovy
Copy link
Copy Markdown
Author

toovy commented Dec 17, 2021

Sorry, never contributed to a project with a CLA :D so I accepted everything, I think now it should be set up correctly. Updated the snapshops. Anything else?

@webpack-bot
Copy link
Copy Markdown
Contributor

@toovy Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@sokra Please review the new changes.

@webpack-bot
Copy link
Copy Markdown
Contributor

Hi @toovy.

Just a little hint from a friendly bot about the best practice when submitting pull requests:

Don't submit pull request from your own main branch. It's recommended to create a feature branch for the PR.

You don't have to change it for this PR, just make sure to follow this hint the next time you submit a PR.

@sokra
Copy link
Copy Markdown
Member

sokra commented Jan 14, 2022

I would merge that PR, but it's conflicting and you didn't gave me permission to edit your branch. So please fix conflict so that can be merged.

Copy link
Copy Markdown
Member

@sokra sokra left a comment

Choose a reason for hiding this comment

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

CI failing, snapshots need to be updated

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants