Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rfc: package aliases #2

Merged
merged 2 commits into from
May 9, 2018
Merged

rfc: package aliases #2

merged 2 commits into from
May 9, 2018

Conversation

zkat
Copy link
Contributor

@zkat zkat commented Apr 26, 2018

No description provided.

@Dwood15
Copy link

Dwood15 commented Apr 29, 2018

Does this allow me to alias a package for use in another library? For example, blessed hasn't been updated in ~3 years. In order to change it to support Windows, I manually fork the library and change one dependency, and publish to npm.

Then, I reference the forked blessed in my own code. This process is kind of silly when the end goal is a drop-in replacement of a dependency of a project that is, itself, a dependency.

@zkat
Copy link
Contributor Author

zkat commented Apr 29, 2018

This RFC explicitly calls out that it is -not- intended to inject packages as transitive dependencies. You can only alter what -your-package will import as a certain name -- transitive deps (and dependents) will not be affected.

@iarna
Copy link
Contributor

iarna commented May 3, 2018

Do we want to allow publication of modules that have aliases as dependencies or do we want to limit this to unpublished (application level) packages?

The advantage of limiting this means that we won't be introducing modules to the ecosystem that users of older package managers (most users) won't be able to install.

If we do that, shoudl there be some sort of escape valve for private publication and private registries? It could be as simple as allowing npm publish $(npm pack), but disallowing npm publish.

@ljharb
Copy link
Contributor

ljharb commented May 3, 2018

It might be a good idea to limit it initially - that way bugs can be worked out, and userland patterns can develop, before exposing it to the wider npm ecosystem.

If it is limited, hopefully it would be either stripped from packages on publish, or better, npm publish would error out if the fields are present.

@Dwood15
Copy link

Dwood15 commented May 4, 2018

The problem with limiting features initially and then expanding those later is that there will always be people + organizations behind in versions of npm.

We could give package maintainers a warning that publishing an alias sets an implicit (or explicit) minimum npm version of currentversion.

Any package which already exists that which adds an alias to their repository should require a maintainer to implement a major version bump if aliasing does break older npm versions.

On a similar note, if aliasing breaks installing packages on older npm versions, engineStrict should be updated/added to the package's package.json if it doesn't already exist.

@zkat
Copy link
Contributor Author

zkat commented May 4, 2018

I fully intend to make this publishable. People already publish packages that are uninstallable on earlier versions of npm. Since this syntax is compatible with Yarn's, I'm even more willing to include it in publishes.

We could help the situation a bit by making sure this patch lands with [email protected] or some other semver-major patch, because it'll make it easier to communicate what version is needed.

@zkat
Copy link
Contributor Author

zkat commented May 4, 2018

(also worth noting: users who publish packages with aliases will likely get plenty of bug reports if their target userbase hasn't upgraded enough, and it's up to them to decide whether their package will be compatible with older npm versions. I'm TOTALLY fine with this and don't consider it ecosystem-splitting because it's a single branch going forward)


There are other, related problems that this RFC will NOT address:

1. replacing packages in transitive dependencies that have different names (`npm i underscore@npm:lodash` will only make it so `require('underscore')` _in your own project_ loads `lodash` instead. Your dependencies will receive their own (nested) version of the actual `underscore` package). This may be covered in the future by an explicit resolutions mechanism, and may be facilitated by this RFC, but is still not part of it.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flattening of deps in npm 3+ seems like it will mean that this boundary isn’t in fact strictly or reliably adhered to. If a transitive dep depends on underscore at a version matching the aliased underscore, wouldn’t it use that one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no. That's exactly what this paragraph is saying. Dependencies are identified by both name and spec version. underscore@npm:[email protected] and [email protected] are entirely different, and incompatible dependencies, so they won't be flattened.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah ok gotcha, so the aliasing transparently applies to the filesystem folder name, but not to the "ideal dep graph" logic?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

both.

@iarna iarna added ratified and removed in-progress labels May 9, 2018
@zkat zkat merged commit adc776e into latest May 9, 2018
@zkat zkat deleted the zkat/rfc-package-alias branch May 9, 2018 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants