bower to npm - angular-patternfly & dependencies #4641
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
|
This pull request is not mergeable. Please rebase and repush. |
|
(current state: done, but |
|
Looks like both According to yarnpkg/yarn#3951, the problem is with the modules, they should mention jQuery as a peerDependency, not dependency. We still can't use provide plugin to override the jquery version being used as long as there are any asset pipeline dependencies, trying to use imports-loader to do the right thing. EDIT: seems to work, updated desciption 👍 |
|
@miq-bot remove_label wip |
|
This pull request is not mergeable. Please rebase and repush. |
|
@miq-bot add_label wip Back to wip :/ Looks like |
same version this one is special because it brings a lot of dependencies
angular-patternfly depended on angular-bootstrap, thus moving the require to webpack-land, to pick the new recursive dependency also moving patternfly itself, because related
not needed now that patternfly brings its own
moving bootstrap and all remaining bootstrap plugins to npm (all of these are angular-patternfly deps) also moving angular-bootstrap-switch because bootstrap-switch was moved
even though patternfly (or angular-patternfly) depend on these packages, for some reason they end up installed *under* patternfly/node_modules so only patternfly can require those => adding as an explicit dep so that we can too
no longer needed, we literally have only 2 bower deps remaining
both of these modules are missing jquery in peerDependencies, causing yarn to provide a separate copy of jquery in nested node_modules that means they register themselves on the wrong jquery instance and are not available overriding the exports shim to cause these to always use window.jQuery
for some packages, we need to disable their amd/commonjs boilerplate we're using imports-loader to do that but without WEBPACK_EXCLUDE_NODE_MODULES=1, this would not work because babel-loader would remove the changes this makes it possible for babel-loader and imports-loader to coexist, fixes bootstrap-select & bootstrap-datepicker
|
Moved Still wip: |
|
Fixed |
when run under babel-loader, `(function() {})()` gets {} as this, instead of window
working around that
added the problematic modules to a spec, to prevent webpack-related regressions
|
Checked commits https://github.com/himdel/manageiq-ui-classic/compare/8cb0164256223017fc5ff00c189a0d6378a82a98~...81478bbd0440bab09b33a00a11649386529d158e with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
|
@miq-bot remove_label wip I hope :) |
|
Tested as said in description:
LGTM 👍 I didn't find anything broken or errors in console. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Moving
angular-patternflyto npm.Issue #3734
This PR affects the following extra dependencies (moved from bower to npm, or from asset pipeline to webpack) - names and how to test each:
/network_router/new, see the "Administrative State" field)angular.element('.ng-scope').injector().get('Notifications')angular.element('.ng-scope').injector().get('$uibTooltip')$.fn.modal$.fn.datepicker$.fn.selectpicker$.fn.bootstrapSwitch$.fn.TouchSpinwindow.c3window.d3$.fn.datetimepicker(you can use
bower pruneto ensure these no longer exist inbower_components)This will cause a console warning:
WARNING: Tried to load angular more than once.That's inevitable until we upgrade angular-patternfly to a newer version - the current one depends on angular 1.5 exactly, but we're using 1.6 already.
(1.6 gets loaded first, then angular-patternfly tries to load 1.5 which outputs the warning and stops, so 1.6 does end up being used anyway)