10000 move jquery and jquery-migrate to npm/webpack by silverwind · Pull Request #9813 · go-gitea/gitea · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

move jquery and jquery-migrate to npm/webpack #9813

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 21, 2020
Merged

move jquery and jquery-migrate to npm/webpack #9813

merged 1 commit into from
Jan 21, 2020

Conversation

silverwind
Copy link
Member

Currently, this needs to be its own chunk because fomantic depends on jQuery being present. The next step is to move fomantic to webpack too after which we can combine the index,fomantic and jquery files into one.

jquery-migrate is still neccessary because our ancient version of Dropzone seems to break without it. I imagine it can be removed after a Dropzone upgrade.

@codecov-io
Copy link
codecov-io commented Jan 17, 2020 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (master@0e8b27a). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #9813   +/-   ##
=========================================
  Coverage          ?   42.31%           
=========================================
  Files             ?      605           
  Lines             ?    79313           
  Branches          ?        0           
=========================================
  Hits              ?    33561           
  Misses            ?    41610           
  Partials          ?     4142

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0e8b27a...963dfa8. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 17, 2020
@silverwind
Copy link
Member Author

Regarding future interaction between jQuery and fomantic: Problem is jQuery does not set globals when loaded via webpack but they must be present for fomantic to load correctly. I imagine some experimenting with one of these might get it to work, but it will be a bit tricky:

lafriks
lafriks reviewed Jan 17, 2020
View reviewed changes
@lafriks lafriks added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Jan 17, 2020
@lafriks lafriks added this to the 1.12.0 milestone Jan 17, 2020
@silverwind
Copy link
Member Author
silverwind commented Jan 17, 2020 •
edited
Loading

I think I might opt to use copy-webpack-plugin for jQuery to deal with the future fomantic situation, so this should be considered WIP for now.

@lafriks
Copy link
Member
lafriks commented Jan 17, 2020

I think better currently leave as it is now, and when moving fomantic than we can think about copy-webpack-plugin

@silverwind
Copy link
Member Author

Agree, that can be done later, so this should be good to merge for now.

@silverwind
Copy link
Member Author

Rebased and included a tweak to webpack which eliminates asset size warnings currently present on master.

This is good to go from my side. Any reviews?

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jan 20, 2020
@lunny
Copy link
Member
lunny commented Jan 21, 2020

Please resolve the conflicts.

@silverwind
Copy link
Member Author

Rebased and squashed.

@silverwind
move jquery and jquery-migrate to npm/webpack
963dfa8 
Currently, this needs to be its own chunk because fomantic depends
on jQuery being present. The next step is to move fomantic to webpack
too after which we can combine the index,fomantic and jquery files into
one.

jquery-migrate is still neccessary because our ancient version of Dropzone
seems to break without it. I imagine it can be removed after a Dropzone
upgrade.
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jan 21, 2020
@lafriks lafriks merged commit 2982afe into go-gitea:master Jan 21, 2020
@silverwind silverwind deleted the jquery-npm branch January 21, 2020 19:30
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lafriks lafriks lafriks approved these changes

@guillep2k guillep2k guillep2k approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Milestone
1.12.0
Development

Successfully merging this pull request may close these issues.
6 participants
@silverwind @codecov-io @lafriks @lunny @guillep2k @GiteaBot
0