Sweetalert2: Built files ("./dist") in version-control

[zenflow]

Built files are useless redundant noise in the git diff (a lot of it if you can never commit them), and generally make using Git more difficult for everyone

Listing them in the .gitignore does not have any effect because it does not make any sense; garbage in, garbage out.

Built files have no place in version-control (aka source-control, as in source, not build-control).

Package managers are our build-control; they are what we use to distribute/share build-code.

[limonte]

Built files are useless redundant noise in the git diff (a lot of it if you can never commit them), and generally make using Git more difficult for everyone

There's assume-dist-unchanged npm task, would this be helpful for contributors to auto-run assume-dist-unchanged once they run gulp watch? Please leave your opinion here: https://github.com/sweetalert2/sweetalert2/pull/823

Listing them in the .gitignore does not have any effect because it does not make any sense; garbage in, garbage out.

Built files have no place in version-control (aka source-control, as in source, not build-control).

Package managers are our build-control; they are what we use to distribute/share build-code.

Related issues from the related project :)

• https://github.com/t4t5/sweetalert/issues/733
• https://github.com/t4t5/sweetalert/issues/748

I prefer to be less strict about VCS definition and be more accessible for all users and workflows.

@jamiesonbecker has listed several valid arguments for keeping the dist folder with both unminified and minified versions.

[zenflow]

t4t5/sweetalert#748 does not apply to this repo, since our unobfuscated dist files are freely accessible on the npm package distribution registry (that is where unpkg.com gets packages from), unlike @t4t5's sweetalert which only publishes the _minfied/obfuscated_ dist files. Removing files from source-control does not mean they are removed from the package distribution.

I prefer to be less strict about VCS definition and be more accessible for all users and workflows.

Our distribution files are already perfectly accessible for all users and their workflows, minified or unminified, on the condition that the correct dist files are always present when you npm publish or yarn publish. (You should guard against human error regarding that condition, by running the build automatically before publishing, via scripts.prepublish in package.json)

(edit: Correction: If we removed the dist from Git, the Bower workflow would not supported anymore. Thanks for making this point @toverux)

I think/hope I made it clear that something that could use improvement is this package's internal workflow. I would not complain, but like I said on t4t5/sweetalert#748...

Keeping build/dist files in source-control is really going against widespread convention and causes pain with Git tooling. Check out the setup of the React project for instance. It is extremely standard, in that dist files are included in the package distribution on npm (check the umd folder) and not included in the source-code repository on Github.

We can easily match that exactly.

[zenflow]

I'm not sure how many of the concerns in t4t5/sweetalert#733 I need to address..

I don't even know what @jamiesonbecker is concerned about here:

the virtues vs tradeoffs of maintaining backwards compatibility with previous versions

Using unpkg.com and CLI package managers like npm, bower, yarn, all give you good management over this kind of thing. It's what they do.

[toverux]

Isn't bower downloading directly from GitHub? In this case, the users wouldn't have the minified versions. What about the CDNs?

Publishing build artifacts on Git was common before npm. It is also seeked by users who don't use a package manager and just click on "Download ZIP" (been there).
I hate dist/ files too and you can see that all my JS repos are exempt from that, but they are targeting Angular or Node, where people always use npm. Dist files regularly cause pain here and require more work. But that's still a common distribution system for _this type_ of frontend libraries.

I'm not against removing them from Git because it's just better for us and our users, but I'm worried this will impact some of them, as well as (if I'm right) the Bower/CDN distributions. It's not just a "chore(package)" change in our package.json.

[jamieson99]

Ok, don't want to really judge how other people architect their build processes. There is a big world beyond NPM. Even beyond the security issues, there are build and speed issues with NPM (and bower, or any other flavor of the month) in some environments. The opportunity cost of engaging in an argument about "best" practices was just too high or waiting for a show-stopping bug for us to even be responded to, as well as rewriting all of our modal calls to meet the new incompatible API for v2. In the end, we wished that dev team well and it was just faster to switch from SA1 to our own simple/custom modal library instead of an alternative (not this project).

[limonte]

I vote for removing the dist folder in the next major, as it's a breaking change for our Bower users.

[jamieson99]

Just a suggestion - agreed completely that dist doesn't need to be in the repo, but adding dist style minified and unminified files as links to the README might be really helpful for some of your users. (Github offers binary release functionality that supports this.)

Most Javascript libraries are usually just a single CSS and JS, with maybe a few options for CSS themes or something. Usually, if someone says just add this <script> and <link> (CSS) to your html, I just type wget, and paste the URLs; this pulls down the two files into our repo, and they're automatically included in our builds from then on. Fast and easy and works with all types of JS, packaged or not. When most things can be boiled down to just a file or two, a wget works great.

[zenflow]

Isn't bower downloading directly from GitHub?

Ah, so you are right. I was under the false impression that you had to bower publish your packages.

As for CDN, are there any that we use/recommend besides unpkg.com? @limonte

Yes, like @toverux expressed, I too feel bad about leaving Bower users "out in the cold". Assuming we all agree that the dist files should be removed from Git in the next major, I have two ideas on how to help them out:

1. In the release notes, include a note (a) encouraging them to migrate to Yarn and Webpack, using Bower's own migration guide which has instructions for using a neat looking package called bower-away, and (b) suggesting that they can always grab the dist off unpkg.com
2. Really go out of our way like jQuery does, pushing our dist to a separate repo in the release process, to be published on Bower.

[zenflow]

It looks like Bower users have the option to do this:

"dependencies": {
  "sweetalert2": "https://unpkg.com/[email protected]/dist/sweetalert2.js"
}

Reference: https://github.com/sheerun/bower-away#edge-cases

Maybe that should be the suggested method for Bower users.

[zenflow]

We are connected with some Bower users through various issues in this repo.. I am wondering if we should try to get some of them involved in this discussion

[zenflow]

Better yet,

"dependencies": {
  "sweetalert2": "v7.3.5"
}

The v7.3.5 is a tag name, for a commit in a dist branch.

Dist files won't be a drag if they're in Git as long as they aren't in master or any of the development branches.

[zenflow]

We will be implementing this in the current major version. See PR #840

[limonte]

@zenflow I tested npm run release twice and both times got this error:

Pushing to Github...
    (pid:13813) [cmd]   git push origin dist:dist --tags
    (pid:13813) [ERR]   To [email protected]:sweetalert2/sweetalert2.git
    (pid:13813) [ERR]      db548db..9774a3c  dist -> dist
    (pid:13813) [ERR]    * [new tag]         v7.8.5 -> v7.8.5
    (pid:13813) [ERR]    ! [rejected]        v6.6.10 -> v6.6.10 (already exists)
    (pid:13813) [ERR]    ! [rejected]        v6.6.8 -> v6.6.8 (already exists)
    (pid:13813) [ERR]    ! [rejected]        v6.6.9 -> v6.6.9 (already exists)
    (pid:13813) [ERR]    ! [rejected]        v6.7.0 -> v6.7.0 (already exists)
    (pid:13813) [ERR]    ! [rejected]        v6.9.1 -> v6.9.1 (already exists)
    (pid:13813) [ERR]   error: failed to push some refs to '[email protected]:sweetalert2/sweetalert2.git'
    (pid:13813) [ERR]   hint: Updates were rejected because the tag already exists in the remote.
    (pid:13813) [ERR]
    (pid:13813) [out]
{ Error: Command failed: /bin/sh -c git push origin dist:dist --tags
To [email protected]:sweetalert2/sweetalert2.git
   db548db..9774a3c  dist -> dist
 * [new tag]         v7.8.5 -> v7.8.5
 ! [rejected]        v6.6.10 -> v6.6.10 (already exists)
 ! [rejected]        v6.6.8 -> v6.6.8 (already exists)
 ! [rejected]        v6.6.9 -> v6.6.9 (already exists)
 ! [rejected]        v6.7.0 -> v6.7.0 (already exists)
 ! [rejected]        v6.9.1 -> v6.9.1 (already exists)
error: failed to push some refs to '[email protected]:sweetalert2/sweetalert2.git'
hint: Updates were rejected because the tag already exists in the remote.

Could you please make the next release and validate if this happens to you as well?

[zenflow]

Could you please make the next release and validate if this happens to you as well?

Hmm, yeah you got it. Let me know here when there's a release to make.

In the meantime, what happens if you switch to the dist branch and run git push origin dist:dist --tags? On my system I have "git version 2.14.1.windows.1" and it prints "Everything up-to-date".

[zenflow]

Nevermind, I can do a patch release now since I just merged #868. Making my first release of SweetAlert2! Woot! 😃

[zenflow]

Hmm.. I did not get that problem:

Pushing to Github...
    (pid:7704)  [cmd]   git push origin dist:dist --tags
    (pid:7704)  [ERR]   To https://github.com/sweetalert2/sweetalert2.git
    (pid:7704)  [ERR]      9774a3c..94d428c  dist -> dist
    (pid:7704)  [ERR]    * [new tag]         v7.8.6 -> v7.8.6
    (pid:7704)  [out]
    (pid:7704)  [ERR]
Switching back to "master" (so you can continue to work)...

Hopefully we can reproduce on your system by just running git push origin dist:dist --tags...

[zenflow]

@limonte If you can reproduce with that command, does this fix it? https://stackoverflow.com/a/39534896

[limonte]

Update:

• I installed the latest version of Git:

> git --version
git version 2.16.1

• Tried to make another patch release and got the same issue:

Pushing to Github...
    (pid:7032)  [cmd]   git push origin dist:dist --tags
    (pid:7032)  [ERR]   To github.com:sweetalert2/sweetalert2.git
    (pid:7032)  [ERR]    * [new tag]         v7.8.7 -> v7.8.7
    (pid:7032)  [ERR]    ! [rejected]        dist -> dist (non-fast-forward)
    (pid:7032)  [ERR]    ! [rejected]        v6.6.10 -> v6.6.10 (already exists)
    (pid:7032)  [ERR]    ! [rejected]        v6.6.8 -> v6.6.8 (already exists)
    (pid:7032)  [ERR]    ! [rejected]        v6.6.9 -> v6.6.9 (already exists)
    (pid:7032)  [ERR]    ! [rejected]        v6.7.0 -> v6.7.0 (already exists)
    (pid:7032)  [ERR]    ! [rejected]        v6.9.1 -> v6.9.1 (already exists)
    (pid:7032)  [ERR]   error: failed to push some refs to '[email protected]:sweetalert2/sweetalert2.git'
    (pid:7032)  [ERR]   hint: Updates were rejected because the tip of your current branch is behind
    (pid:7032)  [ERR]   hint: its remote counterpart. Integrate the remote changes (e.g.
    (pid:7032)  [ERR]   hint: 'git pull ...') before pushing again.
    (pid:7032)  [ERR]   hint: See the 'Note about fast-forwards' in 'git push --help' for details.

In the meantime, what happens if you switch to the dist branch and run git push origin dist:dist --tags? On my system I have "git version 2.14.1.windows.1" and it prints "Everything up-to-date".

Here's the output of git push origin dist:dist --tags when I'm on dist branch:

[dist] }> git push origin dist:dist --tags
To github.com:sweetalert2/sweetalert2.git
 ! [rejected]        dist -> dist (non-fast-forward)
 ! [rejected]        v6.6.10 -> v6.6.10 (already exists)
 ! [rejected]        v6.6.8 -> v6.6.8 (already exists)
 ! [rejected]        v6.6.9 -> v6.6.9 (already exists)
 ! [rejected]        v6.7.0 -> v6.7.0 (already exists)
 ! [rejected]        v6.9.1 -> v6.9.1 (already exists)
error: failed to push some refs to '[email protected]:sweetalert2/sweetalert2.git'
hint: Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart. Integrate the remote changes (e.g.
hint: 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.

I guess here's the fix for this particular issue: https://github.com/sweetalert2/sweetalert2/pull/875

If you can reproduce with that command, does this fix it? https://stackoverflow.com/a/39534896

I can't find the --tags option in man git pull, probably this is an outdated answer.

[limonte]

    (pid:7032)  [ERR]    ! [rejected]        v6.6.10 -> v6.6.10 (already exists)
    (pid:7032)  [ERR]    ! [rejected]        v6.6.8 -> v6.6.8 (already exists)
    (pid:7032)  [ERR]    ! [rejected]        v6.6.9 -> v6.6.9 (already exists)
    (pid:7032)  [ERR]    ! [rejected]        v6.7.0 -> v6.7.0 (already exists)
    (pid:7032)  [ERR]    ! [rejected]        v6.9.1 -> v6.9.1 (already exists)

I believe something is wrong with these local tags in my repo, I will delete the local repo folder and clone it from the origin to have the fresh repo.

[limonte]

I believe something is wrong with these local tags in my repo, I will delete the local repo folder and clone it from the origin to have the fresh repo.

Yup, that was my issue. Apologies for being a Git-newbie :) Now it works fine:

[dist] }> git push origin dist:dist --tags
Everything up-to-date

But I believe this is still a valid PR: https://github.com/sweetalert2/sweetalert2/pull/875, please take a look when you have time, thanks a lot for helping me out with this!