search

yiisoft / yii2-bootstrap

Yii 2 Bootstrap 3 Extension
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
185 stars 193 forks source link

bower to npm in 2.1 #221

Closed alexantr closed 5 years ago

alexantr commented 7 years ago

Since bower got deprecated status will be good idea to replace bower packages (bootstrap and popper.js) with npm packages.

samdark commented 7 years ago

Agree.

schmunk42 commented 7 years ago

Yes, we should do that, this will make the transition to a new asset installation system also much easier.

Related:

schmunk42 commented 6 years ago

Here's a proposal how it could be done in 2.1: https://github.com/yiisoft/yii2-bootstrap/pull/212

It would be nice if the above PR could be merged into 2.1 to move ahead with the transformation.

beroso commented 6 years ago

Bootstrap 4 can be installed via composer too. It's suitable?

samdark commented 6 years ago

Probably yes.

rob006 commented 6 years ago

I think we should stick to one repository for frontend packages to avoid duplicated and incompatible dependencies. TB may be available at packagist, but many widgets that depends on bootstrap does not support installation via composer. If we install bootstrap by composer and package X (which depends on bootstrap) by npm, we will get two different bootstrap packages.

schmunk42 commented 6 years ago

What's the merge/split policy for yii2-bootstrap? For yii2 there we'll have to wait for 2.0.14 before there'll be a removal of bower-assets.

Can we remove the dependencies here in the 2.1 branch or do we have to wait for the core framework?

In 2.1 there should only be a packages.json file for assets, since bower is deprecated and npm won't be manageable well with composer as laid out here.

PS: Bootstrap also requires popper.js for example.

samdark commented 6 years ago

I'd say:

  1. Fix as many issues and merge as many PRs as possible into bootstrap3 (master) branch.
  2. Make a release.
  3. Merge it into 2.1.
  4. Make 2.1 primary branch (alternatively we can make 2.0 branch and move 2.1 into master).
  5. Fix bootstrap4 things.
  6. Release.
schmunk42 commented 6 years ago

The versions are starting to confuse me :)

The question is, if there really should be a 2.1 version of this repo with Bootstrap 4 - how about yii2-bootstrap4? The major version change should not be "hidden". I somehow doubt that this can be easily upgraded. For sure the widgets itself can be, but you usually have a lot more BS-dependent code in your app.

Moreover - and this applies also to other extensions - a 2.1 version of the extension don't have to be necessarily connected to framework version 2.1 - which might be not very obvious.

So a version of this repo which uses a very different asset management and BS4 should rather be a 3.0 or yii2.1-boostrap4:1.0 - confusing ... isn't it?

samdark commented 6 years ago

3.0 sounds OK to me.

rob006 commented 6 years ago

I thought that 2 is fixed as a prefix for version of all yii2 packages. We already have 2.1 releases for packages compatible with Yii 2.0, why use 3.0 in this case?

adiramardiani commented 6 years ago

Agree 3.0 for bootstrap 4 And 2.x for bootstrap 3

rob006 commented 6 years ago

3.0 release contradicts with existing versioning policy: https://github.com/yiisoft/yii2/blob/master/docs/internals/versions.md

samdark commented 6 years ago

That's framework version policy.

rob006 commented 6 years ago

All official extensions seems to follow it. It will be really confusing, if we make exception only for one extension.

samdark commented 6 years ago

What do you propose instead?

rob006 commented 6 years ago

Release it as 2.1.

schmunk42 commented 6 years ago

Bower support has been dropped for v4, btw: https://github.com/twbs/bootstrap/pull/24590, https://github.com/twbs/bootstrap/pull/23568

schmunk42 commented 6 years ago

I took a deeper look on the asset installation procedure with npm-assets with AP and CAP, and found rather worrying things.

As outlined in https://github.com/yiisoft/yii2/issues/14862 npm is capable of installing multiple version of the same package, which leads to (unsolvable) errors with some packages, like ie. mermaid, see also https://github.com/hiqdev/asset-packagist/issues/69

The same issue pops also up in this combination: 

    "require": {
        "npm-asset/selectize.js": "0.12.*",
        "npm-asset/fastlog": "1.*"
    },

The unsolvable dependency is minimist, which has over 9000 (not a joke) dependents. If there's one constraint mismatch, you won't be able to install.

Wan't it even worse? Checkout https://www.npmjs.com/package/lodash - over 63.000 dependents, like 10% of the ecosystem.

And nobody will care about this problem, since it isn't one when using npm!

Moreover, with npm many packages do not have dist files in the repo all-the-time, ie. if you get dev-master of a package like mermaid, you do not have any usable files at all.

That's not all, when installing npm-asset/bootstrap:4.* you won't get the one dependency you actually really need: popper.js, since it's a peerDependency in npm.

TL;dr

We should not transistion to npm-asset handled by composer in any way (at least by default). This will just show bad-practice to developers - even if this is a very painful decision.

We should remove all npm-asset/... from composer.json files.

This will break some things in the first place, like mentioned from @klimov-paul in https://github.com/yiisoft/yii2/issues/14862#issuecomment-369224200 - but IMHO it is better to break things now and require npm/yarn in this early development stage of 2.1, than to ignore this and worry later.

It's doomed to failure, if it stays like it is.

samdark commented 6 years ago

Makes sense. Seems we have to do it :(

fcaldarelli commented 6 years ago

@schmunk42 What will change by developer point of view using directly npm instead npm-asset?

schmunk42 commented 6 years ago

You'll need to install npm or yarn. But that's the only con IMHO. The native solution is much faster and less error-prone.

fcaldarelli commented 6 years ago

@schmunk42 It is ok. This is an irrilevant con if compared to pros.

alexantr commented 6 years ago

It would be great to use npm instead workarounds like asset-packagist.

Just one more command.

composer require yiisoft/yii2-bootstrap
npm i --save bootstrap
schmunk42 commented 6 years ago

@alexantr Have a look https://github.com/fxpio/foxy - it would (optinally) automate this process

alexantr commented 6 years ago

@schmunk42 It seems nice

Foxy retrieves the list of all Composer dependencies to inject the asset dependencies in the file package.json, and leaves the execution of the analysis, validation and downloading of the libraries to NPM or Yarn. Therefore, no VCS Repository of Composer is used for analyzing the asset dependencies, and you keep the performance of native package manager used.

cebe commented 6 years ago

As outlined in yiisoft/yii2#14862 npm is capable of installing multiple version of the same package, which leads to (unsolvable) errors with some packages, like i.e. mermaid, see also hiqdev/asset-packagist#69

the PHP based stack is not meant to provide the full functionality that npm has. It is meant to be used for simple setups, e.g. if you fetch bootstrap, jquery and 1-2 other libs for a simple site or application. If your application depends on a package that only works with native npm, you should be able to switch the stack and use npm directly.

Given the mess that npm and javascript in general currently is, we need a way to use Yii for developing a website without the need to deal with all the complexity introduced by the nodejs/npm stack.

schmunk42 commented 6 years ago

Given the mess that npm and javascript in general currently is, we need a way to use Yii for developing a website without the need to deal with all the complexity introduced by the nodejs/npm stack.

I agree to this statement, but on the other hand we also need to show the "correct" way to the developer. What you've said is feasible for jQuery & Bootstrap, because there are core AssetBundles for those currently - and you can depend on them.

But for anything else you run into problems, take https://github.com/Insolita/yii2-adminlte-widgets for example - they require AdminLTE, but there's no official core asset-bundle for it, so you can't depend on anything (and if you would, some people might want to exchange the PHP asset-bundle). It also makes no sense to provide your own, because in this case there is ie. our widely used extension. A dependency would only make sense on a CSS/JS level.

While I don't support a CDN by default, how about the following (would it be possible?):

Could we reconfigure @node_modules to http://cdn.node-modules.org?

samdark commented 6 years ago

Using CDN by default is a no go.

schmunk42 commented 6 years ago

Sure, the question is, if a developer could install an application template - and then manually set the mentioned alias, like moving from local filesystem to CDN, just by changing the "include path".

An additional alternative would be an auto-built composer package, with the required JS/CSS files.

samdark commented 6 years ago

Isn't it what we have now with asset-packagist? It's auto-built composer package.

schmunk42 commented 6 years ago

For simple setups it looks feasible with AP, but the state of npm-package-dependencies seems much worse than bower:

peer dependency (see also https://github.com/hiqdev/asset-packagist/issues/77)

"jam" dependency - whatever that is :)

no dependency

dependency

looks like most have a correct dependency

has a dependency, but not in a release

alexantr commented 6 years ago

Need a composer repository like asset-packagist but only with npm dist builds and normalized dependencies 😉

schmunk42 commented 6 years ago

@alexantr Related? https://github.com/hiqdev/asset-packagist/issues/67

A problem with npm is that you can't rely on getting dist files for several commits. What do you mean with normalized dependencies? Might make sense to continue this point at asset-packagist.

alexantr commented 6 years ago

Just want to use npm simply like bower.

schmunk42 commented 6 years ago

Just want to use npm simply like bower.

Yeah :smiley_cat: I would love to do that also.

Now that bower is gone, I miss it. Bower was actually ideal for the way assets were handled in Yii2. But it is like it is, we'll have to live with npm and its issues.