Skip to content

Removed jQuery from auto provided variables in webpack config #694

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

Merged
merged 1 commit into from
Dec 19, 2017

Conversation

mathop
Copy link
Contributor

@mathop mathop commented Nov 7, 2017

window.jQuery is already defined in autoProvidejQuery function, there's no need to override:

https://github.com/symfony/webpack-encore/blob/v0.9.1/lib/WebpackConfig.js#L271

@javiereguiluz
Copy link
Member

You are right ... but when we did the transition to Webpack Encore, this didn't work. In fact, the autoProvidejQuery() option is the only Encore feature that never works for me. I'm sure it's not Encore's fault but legacy jQuery code fault ... but still, we must be careful. Could you please double check that everything works in all browsers after removing this? Thanks!

@stof
Copy link
Member

stof commented Nov 7, 2017

@javiereguiluz at some point, Encore reverted this change, before adding it again (after figuring out why something was not working). I think this explicit stuff might have been added in the demo during the reverting phase.

In fact, the autoProvidejQuery() option is the only Encore feature that never works for me. I'm sure it's not Encore's fault but legacy jQuery code fault ... but still, we must be careful.

there is 2 kind of legacy code handling:

  • legacy libraries not supporting CommonJS require to get jquery, which are part of your webpack-processed assets. This is the case that autoProvidejQuery is covering
  • legacy application code in your project, which is not processed by webpack (for instance inline JS in your template) which expects to have a jQuery global variable (with plugins registered in it). autoProvidejQuery is not covering this case (autoProvideVariables is not either).

@mathop
Copy link
Contributor Author

mathop commented Dec 4, 2017

@javiereguiluz Sorry for the delay, it seems working fine with different browsers.

@javiereguiluz
Copy link
Member

Thanks @mathop.

@javiereguiluz javiereguiluz merged commit 9860ffd into symfony:master Dec 19, 2017
javiereguiluz added a commit that referenced this pull request Dec 19, 2017
…fig (mathop)

This PR was merged into the master branch.

Discussion
----------

Removed jQuery from auto provided variables in webpack config

`window.jQuery` is already defined in `autoProvidejQuery` function, there's no need to override:

https://github.com/symfony/webpack-encore/blob/v0.9.1/lib/WebpackConfig.js#L271

Commits
-------

9860ffd Removed jQuery from auto provided variables in webpack config.
@mathop mathop deleted the webpack-jquery-duplicated branch December 19, 2017 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants