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

replace component installer #4558

Merged
merged 3 commits into from
Dec 7, 2020
Merged

replace component installer #4558

merged 3 commits into from
Dec 7, 2020

Conversation

craigh
Copy link
Member

@craigh craigh commented Dec 5, 2020

replace component installer with oomphinc/composer-installers-extender
remove RequireJs config generator (ping @Kaik)
remove forced use of composer 1 and allow composer 2 in travis build and github actions

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Fixed tickets -
Refs tickets -
License MIT
Changelog updated yes

remove RequireJs config generator (ping @Kaik)
remove forced use of composer 1 and allow composer 2 in travis build and github actions
@craigh craigh added this to the 3.1.0 milestone Dec 5, 2020
@craigh craigh requested a review from Guite December 5, 2020 22:38
@craigh
Copy link
Member Author

craigh commented Dec 5, 2020

@Guite @Kaik This removes RequireJs stuff. Since I don't know what it was doing in the first place, I don't know why it was there. It was using classes from component installer (and Assetic !!) so if we need it, then we have to come up with a better way. please let me know.

@Guite
Copy link
Member

Guite commented Dec 6, 2020

Did you also consider https://github.com/fxpio/foxy ? (see "Alternatives" section at #4032 ...)

@craigh
Copy link
Member Author

craigh commented Dec 6, 2020

Did you also consider https://github.com/fxpio/foxy

yes, I looked into it. oomphinc/composer-installers-extender is a smaller change and easier for the moment. Foxy could be used later maybe.

@craigh craigh merged commit 9a056a0 into master Dec 7, 2020
@craigh craigh deleted the _remove_component_installer branch December 7, 2020 22:05
@Kaik
Copy link
Contributor

Kaik commented Dec 11, 2020

Hard to say now why RequireJs was required by component installer, or why it generated config for it. AFAIR it wasn't even configured properly.
SF and we use Webpack now.

@Kaik
Copy link
Contributor

Kaik commented Dec 11, 2020

ah ok so, it was an alternative way to handle component js/css we used the simple way (we just hardcoded all assets). These were also "bundled" via requireJS. So instead of many hardcoded tags you use one tag the same everywhere and in your js code you can just call required "module" (JQuery etc...) and requireJS will provide it.
Webpack does the same (and more) but again we are not using it this way (actually looks like we are not using Webpack Encore at all)
Anyway requireJS can be safely removed.

@Guite
Copy link
Member

Guite commented Dec 11, 2020

Yes, requireJS is not needed.
Encore becomes more interesting now that Stimulus is going to provide a Flex hook so bundles can be incorporated into Webpack.

craigh added a commit that referenced this pull request Dec 28, 2020
symlink jqueryui to jquery-ui for BC. refs #4558
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants