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

Asset management in Yii 2.1 #7

Closed
schmunk42 opened this issue Sep 20, 2017 · 29 comments
Closed

Asset management in Yii 2.1 #7

schmunk42 opened this issue Sep 20, 2017 · 29 comments
Milestone

Comments

@schmunk42
Copy link

This issue is about asset-management in general for Yii 2.1. While there's already a checked box for "Make fxp composer plugin optional. Describe how to use asset packagist instead." on https://github.com/yiisoft/yii2/wiki/Plan-for-next-major-versions#client-side I still think we need to elaborate on this topic.

Current situation with bower

In the current state the Yii ecosystem mainly uses bower-assets for installing client side scripts (or CSS) packages. This can either be done using https://github.com/fxpio/composer-asset-plugin (cap) or https://github.com/hiqdev/asset-packagist (ap).

One of the main issues with this approach is that bower is deprecated yiisoft/yii2#14297, and projects start dropping bower support (eg. https://news.ycombinator.com/item?id=10891324, lodash/lodash#2206 (comment), chartjs/Chart.js#3081 {they brought it back, but still...}).
While this currently does not affect "near-core" packages, like jquery or bootstrap, there should be a safe migration path to npm in planning.

Yii 2.1 should also deprecate bower usage, while still having the possiblity to use 2.0 extensions with bower via a separate ap-URL, see hiqdev/asset-packagist#67

Moving to npm

While both tools mentioned above (cap, ap) are able to work with npm packages, there might (I'd say will) be huge issues, when switching to npm packages (ie. hiqdev/asset-packagist#69, hiqdev/asset-packagist#68) One problematic dependency is minimist - a lib with over 10M downloads ... a week(!). A dependency of over 8000 other projects; details are outlined here - it boils down to flat (composer, bower) vs. tree (npm) installation.

That's also one reason why foxy was created. The basic idea is to use native tooling for client side packages (npm/yarn/webpack - currently), along with package.json files, while still having a minimal connection for dependency resolution between server and client side packages, for ie. widgets or REST API connectors.
There are also discussions about smooth transitions for older packages, see fxpio/foxy#8

The main question here is: How will Yii2 core-extensions require assets? (should be best-practice)

Using npm-asset/package looks convenient in the first place, but as layed out above, this might fail even in smaller setups - IMHO we currently do not see issues popping up about this, because only few people use npm-asset. Another contra is that composer.json is "polluted" with a package-syntax which does not belong there.
[update] Another con of ap is that every not-registered package will throw an error unless manually registered on their website, which can be pretty cumbersome on projects with a lot of dependencies.

A con using foxy + packages.json requires node + npm or yarn to be installed.

[update] There's also a proposal to use a CDN for delivering assets by default, which is not a viable solution IMHO, see yiisoft/yii2#8452 (comment)

Conclusion

The ideal solution would be something where the developers can freely choose which solution they want to use.

Feedback highly appreciated. CC: @yiisoft/core-developers @mikehaertl @tonydspaniard

More related issues/PRs:

@tunecino
Copy link

tunecino commented Sep 20, 2017

JS is crazy, it is growing too fast and libs are getting deprecated/replaced too soon. I think you are right, IMHO 2.1 release is a good opportunity to get rid of as much JS dependencies as possible and maybe figure out a more flexible way to let developer implement what to use using whatever dependency tool he needs.

@tom--
Copy link

tom-- commented Sep 20, 2017

It might help to think in terms of what would work well for the demo apps.

IMO we want yii2-app-basic to have an easy quick-start but also allow scaling to realistic complex projects. If it would it be acceptable that it has a packages.json in root dir and requires the user to run an npm command then I think we can do it. The npm command runs npm-scripts that run sass, webpack or whatever we want.

This means that Yii takes a bit different direction in that it expects the user to take responsibility for managing asset dependencies and processing separately from Yii.

Npm and packages.json alone is not the right tooling except for rather simple projects and I think this works well for us. a) It means users have freedom to take asset management it in any direction. And b) it nudges them to take responsibility for this freedom sooner rather than later as their projects grow.

@tom--
Copy link

tom-- commented Sep 20, 2017

Btw, to explain myself a bit (or possibly to confuse y'all because I'm going meta:), I have given up hope that DRY is an appropriate goal in the configuration of a Yii project's PHP and non-PHP dependencies. My history in Yii 1.1 and 2.0 led me to conclude that, in the long run, the tooling needed for DRY leads to more, not fewer, maintenance headaches, such as those Tobias diagnosed here and elsewhere.

In practice this means that I now prefer to require developers to "manually" maintain the relationship between their PHP and non-PHP worlds. That is to say, they have to keep two configurations in a working alignment. I believe this is an exception to DRY, which "says that every piece of system knowledge should have one authoritative, unambiguous representation." Yii will serve its users well if it gives them a good start on this road and this is better than involving magic that obscures the inevitable problem.

@samdark
Copy link
Member

samdark commented Sep 21, 2017

  1. The plan to make core asset-free won't change.
  2. Using npm (either directly or via foxy) in an application template is fine as long as these are maintained and widely adopted in JS world.
  3. Requiring npm package.json in extensions that need clientside scripts sounds OK.

@iamgold
Copy link

iamgold commented Sep 30, 2017

Maybe 2.1 could remove bower library that it's only used in registering assets position. I think each engineer has to know which codes or register position depends on bower's library, so I hope it's an optional way, not required for installing.

@samdark
Copy link
Member

samdark commented Sep 30, 2017

Bower isn't used in registering asset position... would you please elaborate?

@iamgold
Copy link

iamgold commented Sep 30, 2017

When programmer registered position using POS_READY, the View component will register jQuery asset bundle into page @see https://github.com/yiisoft/yii2/edit/master/framework/web/View.php#L469, but maybe i am wrong, because I didn't view all lines. ^^

@samdark
Copy link
Member

samdark commented Sep 30, 2017

Yes. That's correct. That has nothing to do with bower though. Ability to register scripts on DOM ready will be moved out of the core in 2.1 or removed altogether.

@schmunk42
Copy link
Author

schmunk42 commented Feb 28, 2018

from yiisoft/yii-jquery#1 (comment) CC: @klimov-paul

How should I know?
It seems you and @schmunk42 have already decided to move towards new asset plugin. If you did, you should start to upgrade infrastructure, including the docs at the core and all asset-related extensions.

There's no decision at all made about any plugin - the only proposal I made is to remove the JS/CSS-
dependencies from composer.json. The goal is that you can choose the way how to install assets.

I would rather use CDN as default asset bundle source to aviod necessity of any plugin or other composer hack usage. This will allow particular extension to function out-of-the-box in most case, while leaving the developer choice in which way he want to install the assets.

CDN is a no-go IMHO, I explained that here: yiisoft/yii2#8452 (comment) - Yii should not use this by default.

I suppose it is too early to perform global asset management changes, like enforcing 'foxy' usage.
The core extensions yii2-gii and yii2-debug should be stripped from asset dependency first. Only after then this matter can be considered.

As said above, there are no-plugins involved or mandatory ones planned.

[addon] We need to start somewhere, I'd say yii2-jquery is the best place to start, it is even deeper linked into the core than yii2-gii or yii2-debug, but they also need work - for sure.

@klimov-paul
Copy link
Member

klimov-paul commented Feb 28, 2018

If asset dependencies are removed from composer.json it means the extension is not functioning after installation. It also means that version constraints checking will be unavailable.
This brings issue #4391 back.
So it seems you have decided that it should be fine, did not you?

CDN is a no-go IMHO

It may have a drawbacks, but in case CDN is absolutely unacceptable as you say, then no official CDN (like https://code.jquery.com/) would ever exist, but they do.
For me using CDN allows avioding of baby-crying issues like 'I have installed yii2-jquery but it does not work' (no assets manually installed) or 'my installation failed: there is no package npm/jquery' (no asset plugin configured).

One way or another each asset-dependent package should have an explicit instructions about possible asset setup. I have already added those:
https://github.com/yiisoft/yii2-jquery/blob/master/docs/guide/assets-setup.md
https://github.com/yiisoft/yii2-bootstrap/blob/2.2/docs/guide/assets-setup.md

They already include the instrutions, which allows to aviod of installation of assets via composer.

The final decision on this matter depends on the status of yii2-debug and yii2-gii. Those are crucial development tools, which almost always have to be installed. We must attempt to make these tools asset independent, so they will not require neither yii2-jquery nor yii2-bootstrap to function.
If it will become so, then the asset installation problem will be localized up to 2 repositories only and can be solved anytime with thier major releases without core to be involved.
In this case we may decide to strip asset dependencies from composer.json for those packages, forcing developer to maintain assets on his own.

However in case JQuery can not be removed from yii2-debug and yii2-gii, we should choose the option, which garantees asset functioning out-of-the-box. Otherwise setup of basic development tools will produce too much pain for the developer.

I do not consider this issue to be a top priority - the top priority is upgrading yii2-debug and yii2-gii stripping assets from them as the core itself is already free of assets. Only after it is finished with some resolution, it would make sense to return to this matter.

@samdark
Copy link
Member

samdark commented Feb 28, 2018

I'm positive that it's possible to remove jQuery dependency from both debug and gii.

@klimov-paul
Copy link
Member

I will believe it when I see it.

@schmunk42
Copy link
Author

schmunk42 commented Feb 28, 2018

If asset dependencies are removed from composer.json it means the extension is not functioning after installation. It also means that version constraints checking will be unavailable.

The big downside, if assets are in composer.json, is that the packages are "polluted" with them and you need some kind of other tooling to install them. Either AP or CAP - or hacks like asset-free repos.

This brings issue #4391 back.
So it seems you have decided that it should be fine, did not you?

Version checking could be handled with foxy, if you want that, but it must not be required for installation. In a nutshell: Install composer packages, install npm packages, in case of an error with npm, revert both.


As laid out in the inital posting, I see no way how an installation of npm packages will be maintainable with AP or CAP in the future, since multiple versions of dependent packages can be installed with npm, see for example hiqdev/asset-packagist#69 - that problem did not exist with bower.

If we keep npm-asset/... in composer.json (for 2.1) the design is broken right from the beginning.
I've researched this topic quite a lot, let me know if I have overlooked something.

@klimov-paul
Copy link
Member

klimov-paul commented Feb 28, 2018

@schmunk42, if you are sure what to do, then do it: open the pull requests with documentation adjustments, which shows how developer can install assets using 'foxy', using NPM client and so on.
Describe pros and cons of different approaches.
Write the UPGRADE notes.
Each asset-dependent package should have those documentation adjsustments.

Simple removal of the lines from composer.json is not a solution. If we pass asset istallation to developer, we need to provide the explicit instructions for him.

Pull requests like yiisoft/yii2-bootstrap#212 can not be considered as a solution: removing package dependencies without providing information about how it should be handled now is actually a bug creation.

@schmunk42
Copy link
Author

@klimov-paul I agree that there has to be documentation etc. but I did not want to step too far ahead ;)

The work in progress was not continued, because issues like yiisoft/yii2#14832 are totally blocking development, since you can't install the core in 2.1 without CAP or AP. The same goes for gii and debug, since they require bootstrap and assets. And continues with application templates, it can not be done all at once.
I forked the repos to circumvent that for inital development, but it's pretty cumbersome to work with, although I could give it a try again.

All proposed changes should be done in 2.1 branches - IMO some smaller breaks here are OK in such an early devleopment stage. I'd also consider it as a bug to require non-mandatory stuff (AP or CAP) otherwise you can not even install.

About prios and PRs

core

  • Since the core will be asset free, I think this one can be merged; [2.1] no assets yii2#14832 as far as I can see no doc updates are required here

jquery

  • since bootstrap depends on it (in 2.0 through BootstrapPluginAsset on core) and gii and debug depend on bootstrap, it should be tackled first

bootstrap

  • next one in the hiearchy

gii, debug

  • since they depend on bootstrap

App Templates

  • those may provide different ways of installing assets

We also need to differentiate the installation of assets for development/testing in the repo - and in an application.

@schmunk42
Copy link
Author

schmunk42 commented Feb 28, 2018

Here's another idea, although it involves a plugin (but in project scope(!)) and maintenance of two files, I'd like to share it.

If you want automatic asset installation without npm, add the following lines to your composer.json:

    "require": {
        "wikimedia/composer-merge-plugin": "~1.4"
    },
    "repositories": [
       {
          "type": "composer",
          "url": "https://asset-packagist.org"
       }
    ],
    "extra": {
        "merge-plugin": {
            "include": [
                "vendor/*/composer.assets.json"
            ]
        }
    },

A package would have to maintain composer.assets.json with the same deps from packages.json in the current format.

Disclaimer: untested but I am pretty confident that it would work and might be a compromise we all can live with. Let me know if this is an option to check out.

@klimov-paul
Copy link
Member

because issues like #14832 are totally blocking development, since you can't install the core in 2.1 without CAP or AP

???
'2.1' branch is already free from asset dependency.
There is a living proof of it - yii2-captcha:
https://github.com/yiisoft/yii2-captcha/blob/master/composer.json#L31-L36
https://travis-ci.org/yiisoft/yii2-captcha/builds/346997276

@schmunk42
Copy link
Author

Great! I didn't notice, even though it was on 2.1 in the repo for 19 hours already :)

btw: You missed the documentation part in the README from my PR to remove the asset plugin, see https://github.com/yiisoft/yii2/pull/14832/files#diff-8db66c46d0073891dfc96775fcf3682c - also the repositories section for AP can be removed.

Could you take care of it, I'll close my PR since it's mostly obsolete and can not be updated easily.

@klimov-paul
Copy link
Member

7cf9116fe6eb755b9cfa4c383f42b7ce401df864

@schmunk42
Copy link
Author

One more general thing about asset-management in Yii 2.1.

We should remove all possiblities to add logic to asset-bundles, like calling init(), they have to be simple data structures only.

@fcaldarelli
Copy link
Member

There could be situations that it could be useful apply some changes. Take specific version of js or css (for debug, eccc..)., isn't it ?

@schmunk42
Copy link
Author

There could be situations that it could be useful apply some changes. Take specific version of js or css (for debug, eccc..)., isn't it?

You mean like using a non-minified version, when YII_ENV_DEV? If yes, I think there should be a separate asset bundle for it. Espcially this can be a real pain when bundling assets, since the asset is bundled by its properties, but it's not initialized during bundling.

@fcaldarelli
Copy link
Member

So do you suggest for development to use a different asset bundle? In this way, I should change also references inside source code, to switch from production asset bundle to development asset bundle.

I don't understard this part:

Espcially this can be a real pain when bundling assets, since the asset is bundled by its properties, but it's not initialized during bundling.

Can you make an example?

@schmunk42
Copy link
Author

Take this asset-bundle for example https://github.com/beowulfenator/yii2-json-editor/blob/master/src/JsonEditorAsset.php

It uses different files when in YII_ENV_DEV (some others use YII_DEBUG btw.) - so you make changes to the non-minified files, even submit a patch to upstream (of the bower/npm-asset) - and when you run it in production you get the old errors, because it hasn't been minified (rebuilt) properly. Happend to us, extremly hard to debug.

So do you suggest for development to use a different asset bundle?

Actually the opposite. If you need to bundle an minify, this is your job, not the one of asset-bundles, which might work differently. An asset bundle should only be able to define files and dependencies.

Another example is this https://github.com/dmstr/yii2-adminlte-asset/blob/master/web/AdminLteAsset.php

If a property is set, include another CSS file. This does not happen during bundling, so you usually get an error, when using the bundled version.

@fcaldarelli
Copy link
Member

It uses different files when in YII_ENV_DEV (some others use YII_DEBUG btw.) - so you make changes to the non-minified files, even submit a patch to upstream (of the bower/npm-asset) - and when you run it in production you get the old errors, because it hasn't been minified (rebuilt) properly. Happend to us, extremly hard to debug.

This could happen also using an asset bundle that defines only files and dependencies; because if you work on a non-minified version and forget to update the minified version, the result is the same.

(about AdminLTE) If a property is set, include another CSS file. This does not happen during bundling, so you usually get an error, when using the bundled version.

In that case, the "skin" property is never set out of AdminLteAsset. I don't understand how could be used. Is that a code that could be removed?

@samdark samdark transferred this issue from yiisoft/yii2 Nov 13, 2018
@samdark samdark transferred this issue from yiisoft/yii-core Nov 13, 2018
@samdark samdark transferred this issue from yiisoft/yii-web Jun 17, 2019
@terabytesoftw
Copy link
Member

I think this is already solved with foxy and hidev-composer-plugin, and you can configure the option setAlternatives in assetmanager by default comes "@npm/node_module", so I think this issue is resolved.

@samdark
Copy link
Member

samdark commented Aug 25, 2019

I think at least documentation should be done.

@terabytesoftw
Copy link
Member

Done.

yiisoft/view#50

@samdark samdark transferred this issue from yiisoft/view Dec 19, 2019
@samdark samdark added this to the 1.0.0-alpha1 milestone Dec 27, 2019
@samdark
Copy link
Member

samdark commented Dec 31, 2019

Docs now give foxy as an example. Application template would use it as well.

@samdark samdark closed this as completed Dec 31, 2019
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

No branches or pull requests

8 participants