dart-lang / pub

The pub command line tool
https://dart.dev/tools/pub/cmd
BSD 3-Clause "New" or "Revised" License
1.04k stars 224 forks source link

Support: 'pub get' without dev_dependencies --only={prod[uction]|dev[elopment]} #2036

Open jonasfj opened 5 years ago

jonasfj commented 5 years ago

When installing dependencies for use in production, you might wish to exclude dev_dependencies.

use-cases

I'm sure there are more use-cases.

I suggest modeling this after npm which supports: npm install --only={prod[uction]|dev[elopment]}.

We could also simply do pub get --production. But --only=... allows for more flexibility in the future.

jakemac53 commented 5 years ago

I don't think --only=development is valid for the pub use case.

The dev dependencies are typically additive - its just additional deps you need for development (usually for testing, building, deploying, etc). We don't require users to duplicate their regular deps in there, and I think you could rarely get a working project by only taking them into account?

I don't know much about node but I think maybe it works for them because they are just adding additional deps to the node_modules directory, instead of the pub case where we would (presumably) be actually removing any previous deps that existed (we create a new .packages and pubspec.lock file).

zoechi commented 5 years ago

@jakemac53 as far as I know there are checks that libraries from packages imported in files from lib/ or /bin are required to be listed in dependencies:

I think it's the other way around. If you have a package in dependencies you don't need to add it to dev_dependencies: if used outside of lib/ or bin/.

jonasfj commented 5 years ago

Just doing --only=production would be fine for now.

It doesn't matter that this is a complicated flag, as it's mostly used in scripts and very specific options.

jakemac53 commented 5 years ago

@jakemac53 as far as I know there are checks that libraries from packages imported in files from lib/ or /bin are required to be listed in dependencies:

Yes, pub checks that now I believe, but I am not sure what that has to do with this?

My point was within for instance your test dir you are allowed today to import any package that is listed under your regular dependencies. You are only required to list under dev_dependencies the additional deps you need for testing.

So, fetching only dev_dependencies would not work for most cases.

I think it's the other way around. If you have a package in dependencies you don't need to add it to dev_dependencies: if used outside of lib/ or bin/.

Correct

Just doing --only=production would be fine for now.

I think that is fine to leave it open for the future for other options (maybe we add other sections etc).

jakemac53 commented 5 years ago

I would maybe consider making this option be --only=public instead of --only=production?

The regular dependencies I think are most accurately described as your public dependencies because its just the deps that you rely on in your public directories (lib/bin).

For production actually I think it would be more likely you need your dev_dependencies? For instance you shouldn't really be shipping your actual app code under lib - that should only be libraries. Your applications should live under some other directory (for web projects this would be the web folder).

EDIT: I realize flutter violates this principle but they are unique in that, and that violates the pub layout conventions.

zoechi commented 5 years ago

@jakemac53

The dev dependencies are typically additive

Sorry missed the dev here

For production actually I think it would be more likely you need your dev_dependencies?

You think bin/ and web/ are not covered by the rules that check that they only import dependencies:?

For instance you shouldn't really be shipping your actual app code under lib

As far as I know only main() needs to be outside lib/ and since a while not even that - to allow

flutter violates this principle

jonasfj commented 5 years ago

For production actually I think it would be more likely you need your dev_dependencies? For instance you shouldn't really be shipping your actual app code under lib - that should only be libraries.

I would naturally expect the entry-point to be in bin/ and listed under executables in the pubspec.

Note: I'm less worried about static analysis and warnings. If you mistype something in your pubspec, you won't get a warning, it just ignores unknown top-level fields :)

jakemac53 commented 5 years ago

You think bin/ and web/ are not covered by the rules that check that they only import dependencies:?

Only bin/ should be checked, as that is a "public" directory, web/ is not a public directory though and it definitely should not be checked (although it could check against dev_dependencies, but that is irrelevant for publishing so I doubt it does).

I would naturally expect the entry-point to be in bin/ and listed under executables in the pubspec.

That would make sense - except that bin/ is actually a public directory and those executables are exposed via pub run to all downstream users (and even precompiled on pub get - which slows down pub get a lot if you unnecessarily ship things there).

I am pretty sure this happens by default and the executables section only allows you to rename things if desired.

jonasfj commented 5 years ago

That would make sense - except that bin/ is actually a public directory and those executables are exposed via pub run to all downstream users....

If my package is distributed via pub and it contains an application then isn't this also the desired behavior? I would expect most applications where this is used to have publish_to: none and live in private github repositories.

I am pretty sure this happens by default and the executables section only allows you to rename things if desired.

That's not obvious from the documentation :)

jakemac53 commented 5 years ago

If my package is distributed via pub and it contains an application then isn't this also the desired behavior?

For such packages yes I think that makes sense - but I don't think these are really the packages that would be "deployed" generally. Typically packages that ship executables tend to be dev_dependencies, in my experience.

I would expect most applications where this is used to have publish_to: none and live in private github repositories.

Yes, I agree that what we are targeting here, application packages that aren't published to pub.

I would expect such packages to have applications under some non-lib top level dir (maybe bin, but could be anything). Those apps could depend on anything from dev_dependencies or dependencies, yet they are what I would consider "production" targets, which is why I don't think "production" is the right name for the mode that ignores dev deps.

jonasfj commented 5 years ago

Hmm, --only=dependencies could also do the trick.. or --exclude=dev_dependencies

If we want to be obvious :)

Note. yarn does --production=true|false.

Hinten commented 2 years ago

Any updates?

natebosch commented 1 year ago

This should be the default behavior when we do a version solve before publishing a package. That version solve should be primarily aimed at ensuring that package will solvable as expected as a pub dependency when the dev dependencies aren't important.

Even if we get https://github.com/dart-lang/pub/issues/2795 it would still be desirable to check that the leaf package has a sensible version solve, then the next package up has a sensible solve given the leaf is published, and so on.

sigurdm commented 1 year ago

This should be the default behavior when we do a version solve before publishing a package. That version solve should be primarily aimed at ensuring that package will solvable as expected as a pub dependency when the dev dependencies aren't important.

Yeah - I have been thinking in that direction also - however wouldn't it be surprising if the version solve ends up different from current pub get? We do a dart analyze as part of the validation, that might end up with different results based on the resolution....

But maybe that is exactly what we want...

@jonasfj do you have an opinion?

natebosch commented 1 year ago

But maybe that is exactly what we want...

I suspect so. If a different pub solve - which is closer to what consumers of the package will see - results in analysis errors, the package author probably wants to know.

natebosch commented 3 months ago

Making a real distinction between transitive dev vs non-dev dependencies, which we get from the solve that ignores dev dependencies, should also drastically strengthen our warning about pubspec overrides.

Currently we ignore overrides of packages mentioned directly in dev_dependencies, but it's possible for that package to also be an indirect production dependency. Checking for overrides of any package in the transitive production pub solve is a more accurate condition for what we are trying to warn against.