Built files ("./dist") in version-control #820

zenflow · 2018-01-13T09:13:58Z

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.

The text was updated successfully, but these errors were encountered:

… in Git, see #820

* Ignore package-lock.json, see 45e59f1 * Unignore [comitted] dist files, since it seems to bring out some bugs in Git, see #820

limonte · 2018-01-13T15:46:23Z

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: #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 :)

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 · 2018-01-13T19:19:01Z

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 · 2018-01-13T19:28:34Z

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 · 2018-01-13T20:09:02Z

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.

perpetual-hydrofoil · 2018-01-13T21:35:48Z

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 · 2018-01-14T11:12:12Z

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

perpetual-hydrofoil · 2018-01-14T17:38:52Z

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 · 2018-01-14T20:38:22Z

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:

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
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 · 2018-01-15T04:11:01Z

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

"dependencies": {
  "sweetalert2": "https://unpkg.com/sweetalert2@7.3.5/dist/sweetalert2.js"
}

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

Maybe that should be the suggested method for Bower users.

zenflow · 2018-01-15T04:13:28Z

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 · 2018-01-15T10:32:01Z

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.

* Remove dist folder, closes #820 * Remove dist folder mentions from contributing guides

zenflow · 2018-01-27T21:05:23Z

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

limonte · 2018-01-31T17:11:28Z

@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 git@github.com: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 'git@github.com: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 git@github.com: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 'git@github.com: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 · 2018-01-31T21:23:41Z

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 · 2018-01-31T21:32:30Z

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

zenflow · 2018-01-31T22:56:03Z

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 · 2018-01-31T23:49:59Z

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

limonte · 2018-02-01T19:16:01Z

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 'git@github.com: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 'git@github.com: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: #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 · 2018-02-02T08:52:07Z

    (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 · 2018-02-02T08:59:31Z

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: #875, please take a look when you have time, thanks a lot for helping me out with this!

zenflow added a commit that referenced this issue Jan 13, 2018

Unignore [comitted] dist files, since it seems to bring out some bugs…

6409598

… in Git, see #820

zenflow added the domain: devel Development-related, affecting contributors label Jan 13, 2018

This was referenced Jan 13, 2018

Closes #816, Update dist files #818

Closed

Two failing tests in my local dev environment #816

Closed

limonte pushed a commit that referenced this issue Jan 13, 2018

Fix up .gitignore (#822)

82ae907

* Ignore package-lock.json, see 45e59f1 * Unignore [comitted] dist files, since it seems to bring out some bugs in Git, see #820

limonte mentioned this issue Jan 13, 2018

chore(gulp-watch): run assume-dist-unchanged #823

Merged

limonte added the needs: breaking change Solving this issue/merging this PR would cause a breaking change label Jan 14, 2018

zenflow mentioned this issue Jan 15, 2018

Rework build and workflow #826

Closed

5 tasks

zenflow added a commit that referenced this issue Jan 21, 2018

Remove dist folder, closes #820

58cd3ac

zenflow added a commit that referenced this issue Jan 21, 2018

Remove dist folder, closes #820

467ef7c

zenflow added a commit that referenced this issue Jan 21, 2018

Remove dist folder, closes #820

3e403b9

zenflow mentioned this issue Jan 23, 2018

Remove dist folder, closes #820 #840

Merged

limonte pushed a commit that referenced this issue Jan 25, 2018

Remove dist folder, closes #820

fcdad1a

limonte pushed a commit that referenced this issue Jan 25, 2018

Remove dist folder, closes #820 (#840)

3e74a42

* Remove dist folder, closes #820 * Remove dist folder mentions from contributing guides

zenflow added a commit that referenced this issue Jan 25, 2018

cherry-picked "Remove dist folder, closes #820 (#840)"

b6b47a3

limonte pushed a commit that referenced this issue Jan 29, 2018

cherry-picked "Remove dist folder, closes #820 (#840)"

b11aab9

limonte closed this as completed in 2196389 Jan 29, 2018

limonte mentioned this issue Feb 1, 2018

fix(release): reset dist to the state of origin/dist #875

Closed

limonte mentioned this issue Jun 3, 2019

GitHub releases missing built files ("./dist" folder) #1606

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Built files ("./dist") in version-control #820

Built files ("./dist") in version-control #820

zenflow commented Jan 13, 2018

limonte commented Jan 13, 2018

zenflow commented Jan 13, 2018 •

edited

zenflow commented Jan 13, 2018

toverux commented Jan 13, 2018 •

edited

perpetual-hydrofoil commented Jan 13, 2018 •

edited

limonte commented Jan 14, 2018 •

edited

perpetual-hydrofoil commented Jan 14, 2018 •

edited

zenflow commented Jan 14, 2018

zenflow commented Jan 15, 2018

zenflow commented Jan 15, 2018

zenflow commented Jan 15, 2018 •

edited

zenflow commented Jan 27, 2018

limonte commented Jan 31, 2018

zenflow commented Jan 31, 2018

zenflow commented Jan 31, 2018

zenflow commented Jan 31, 2018

zenflow commented Jan 31, 2018

limonte commented Feb 1, 2018 •

edited

limonte commented Feb 2, 2018

limonte commented Feb 2, 2018 •

edited

Built files ("./dist") in version-control #820

Built files ("./dist") in version-control #820

Comments

zenflow commented Jan 13, 2018

limonte commented Jan 13, 2018

zenflow commented Jan 13, 2018 • edited

zenflow commented Jan 13, 2018

toverux commented Jan 13, 2018 • edited

perpetual-hydrofoil commented Jan 13, 2018 • edited

limonte commented Jan 14, 2018 • edited

perpetual-hydrofoil commented Jan 14, 2018 • edited

zenflow commented Jan 14, 2018

zenflow commented Jan 15, 2018

zenflow commented Jan 15, 2018

zenflow commented Jan 15, 2018 • edited

zenflow commented Jan 27, 2018

limonte commented Jan 31, 2018

zenflow commented Jan 31, 2018

zenflow commented Jan 31, 2018

zenflow commented Jan 31, 2018

zenflow commented Jan 31, 2018

limonte commented Feb 1, 2018 • edited

limonte commented Feb 2, 2018

limonte commented Feb 2, 2018 • edited

zenflow commented Jan 13, 2018 •

edited

toverux commented Jan 13, 2018 •

edited

perpetual-hydrofoil commented Jan 13, 2018 •

edited

limonte commented Jan 14, 2018 •

edited

perpetual-hydrofoil commented Jan 14, 2018 •

edited

zenflow commented Jan 15, 2018 •

edited

limonte commented Feb 1, 2018 •

edited

limonte commented Feb 2, 2018 •

edited