msurdi / frontend-dependencies

Copies node packages to a directory where your frontend tools will be able to find them
24 stars 5 forks source link

Feature request: Remove Redundancy in package.json #2

Closed FelixFurtmayr closed 6 years ago

FelixFurtmayr commented 6 years ago

Hello, thank you for this package! I need stuff in this direction.

Currently I dislike, that there is redundancy in the package.json - meaning you have to specify your packages once in the dependencys and then also in the frontendDependencys. What do you think of the following syntax:

old version

from https://github.com/msurdi/frontend-dependencies/blob/master/fixtures/package.json

{
  // usual package.json stuff here ...
  "devDependencies": {
    "jquery": "3.1.0",
    "normalize.css": "4.2.0",
    "turbolinks": "5.0.0",
    "shelljs": "0.7.4"
  },
  "frontendDependencies": {
    "target": "static/build",
    "packages": [
      "jquery/dist/*",
      "normalize.css",
      "turbolinks/{src,LICENSE}"
    ]
  }
}

new version - my suggestion

{
  // usual package.json stuff here ...
  "devDependencies": {
    "shelljs": "0.7.4"
  },
  "frontendDependencies": {
     "target": "static/build",
     "packages": {
        "jquery": {
           "version": "3.1.0",
           "src": "jquery/dist/*"
        },
        "normalize.css": {
           "version": "4.2.0",
           "src": "normalize.css"
        },
        "turbolinks": {
           "version": "5.0.0",
           "src": "turbolinks/{src,LICENSE}"
           // any other options can be added here easily
        } 
     },
  } 
}

Comparison

With the new syntax, the npm install would not fetch the dependencies for the frontend - the script would have to do this on postinstall. But this should not be a problem, as the script is already running postinstall.

What do you think?

My Experience and Thoughts

I was not very happy with bower in the past. Just one more source of trouble. Also Webpack etc. are not bad, but do not what I want. Currently I'm managing npm frontend dependencies with multiple grunt tasks: I have to look into package.json, Gruntfile.js in several places and in the index.html - seems ugly to me. One place in package.json and one in index.html seems enough for me.

I had a look at pancake. https://github.com/govau/pancake I consider it overkill. I think my suggested syntax could be extended also with packaging (sass, minifier), but I see no real need for it, as most packages come with a ready to use (compiled and minified, etc.) version of js and css. With http2, the sending of small files is encouraged. Another advantage is, that the browser can cache common libs easy.

Keep it simple stupid. I consider your lib currently as the best way and hope I can add a little value :-)

msurdi commented 6 years ago

Hey! Thanks for such a detailed explanation of your use case.

With the current syntax, what you are specifying in frontendDependencies.packages to be honest, are not packages, but just directories/files patterns within node_modules (that's why I called it packages and not just directories or similar). So I'm not 100% sure you are specifying redundant information, given that the devDependencies/dependencies specify things to install and frontendDependencies.packages specifies source and target directories of files to copy around.

If I understand correctly your proposal, by including version information within frontendDependencies.packages I understand you want to also use this as the source of truth for the packages to be installed. Is that correct? If so, this means that now we need to also take care of fetching/downloading the dependencies, right? Also, as you stated clearly:

With the new syntax, the npm install would not fetch the dependencies for the frontend - the script would have to do this on postinstall. But this should not be a problem, as the script is already running postinstall.

As I understand it, this has the following drawbacks:

And basically this project could end up becoming an alternative to npm, a kind of bower but using package.json. Overall I don't think the trade-offs are really worth it, I don't think that's in the spirit of this package which is to just keep things simple and just copy a couple files around after npm install.

Again, sorry if I misunderstood your point of view, I'm open to discussion.

FelixFurtmayr commented 6 years ago

Hello msurdi,

thank you for your answer. You understood it correct. Sorry if my explanation was unclear.

As we manage more and more projects over npm, I prefer a nice management in the package.json, over technical drawbacks.

"We need to implement a robust way to replicate npm install (handle git, http, registry downloads...) A ton of work" The Work does not look too complexe for me (a few if - else - cases). I would manually run the npm install for every frontend package. But please correct me, if I'm missing something.

npm install (with no args, in package dir)
npm install [<@scope>/]<name>
npm install [<@scope>/]<name>@<tag>
npm install [<@scope>/]<name>@<version>
npm install [<@scope>/]<name>@<version range>
npm install <git-host>:<git-user>/<repo-name>
npm install <git repo url>
npm install <tarball file>
npm install <tarball url>
npm install <folder>

"npm install won't work as expected, remember also it could have been invoked like npm install --ignore-scripts which won't run the postinstall script"

True. But it would be ok for me. I prefer this to having redundancy in my package.json. I think your current script would also not run, if the postinstall script is not executed.

If the solution is good in the end, we might suggest it to the guys of npm. http://blog.npmjs.org/post/101775448305/npm-and-front-end-packaging As this article was from 2014, it think there is some action needed ;-)

It is you decision to make, if you want this in your package or not. Of course I can make a fork, but some discussion might be better than just making another node module.

No matter what you decide, I look forward to your answer :-)

Regards

Felix

msurdi commented 6 years ago

I understand, and taking out of the equation any technical concern about the difficulty of implementing this I might have, this sounds like a very nice idea after a second though.

What I wonder now, is if this looks more like a small feature for NPM (select just X and Y files from this package after installing and place it over there) than implementing the installation steps in this project.

Route 1: Implement what I'm doing here in a fork of NPM and see what happens.... is this aiming too high? probably...

Route 2: Try to implement what you describe here.

What do you think?

I've built this in my free time and I'm not sure how much time I can put on this, but I'm happy to give you write access to create a branch in this project if you don't want to fork to your account so that you can start building a proof of concept

FelixFurtmayr commented 6 years ago

"this sounds like a very nice idea after a second though" Cool :-)

I would see it as feature of npm. As today all popular frontend libs are also in npm, a easy and clean way to handle them would be great.

Right now we should implement it as npm package and suggest it to other people. If the package gets more and more downloads, we can think about including it into npm itself.

"I've built this in my free time" Fine - I'm happy to get your feedback and your permission. I have the need to better manage my projects and can spend time and money on that. If you want to offer some of your time to control the code and do an npm publish, I would just fork it on gitHub and make a pull request. Otherwise I would be happy about write access in gitHub and npm: Then I won't bother you with updates you have to review. What do you prefer?

msurdi commented 6 years ago

Already gave you collaborator access to the repo. I'm on vacation right now but will help you as much as I can as I come back next week.

FelixFurtmayr commented 6 years ago

Hi msurdi,

so far I seem to be to stupid to directly access the repo ;-)

I made a fork and a pull request. Still not bad that way. Making an object out of the frontendDependencies.packages made the code nicer.

More testcase would be a benefit, but please have a look first and inform me, if you are happy with it.

Regards Felix