-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Yarn Flat Mode Attribute in Package.json breaks projects #4823
Comments
Do you have more info on the kind of conflicts that you're seeing in your projects? One thing we've seen in other projects is that tooling-based dev dependencies introduce many conflicts, but there's no need for tooling to be flat. One pattern we're exploring is to make a |
The problem is less about causing actual breakages, and more that developers will simply switch back to npm to avoid the problem. I've already been sent a message by a dev:
|
Chatted on slack with @ChadKillingsworth and @justinfagnani, riffing on some ideas @FredKSchott has been talking about too. We have two constraints:
The core constraints listed above can be fulfilled with something less strict than A package can have at most one transitive dependency on any A These two constraints express our needs, and force the minimum amount of constraints on the rest of a package's dependencies. Currently
|
What about using npm This would require elements to list their dependencies as peers instead of direct dependencies. package.json {
"name": "@polymer/paper-button",
"version": "3.0.0",
"peerDependencies": {
"@polymer/polymer": "^3.0.0"
}
} The downside to this is that a user would need to manually install the dependencies, of which npm would warn them about. npm install --save @polymer/paper-button
npm ERR! peerinvalid The package @polymer/paper-button does not satisfy its siblings' peerDependencies requirements!
npm ERR! peerinvalid Peer @polymer/paper-button@3.0.0 wants @polymer/polymer@^3.0.0
npm install --save @polymer/polymer The above would result in the following dependency tree:
Unique? Yes, as long as elements declare each other as peer dependencies. If another element requires Web Loadable? Yes. All elements and their dependencies are installed by the app and exist in the top tier Another benefit is that this uses npm directly (no third party tool such as yarn) and it allows nested dependencies with other non-frontend npm modules and build tools. Considering the amount of developer intervention that's needed to make flat node modules work with any other build tool, I'd consider the install step to be minimally intrusive. |
This is a general problem with modules, not just with Polymer, so in effect all dependencies would have to be peer dependencies, which mostly defeats the purpose of a package manager. |
But aren't peer dependencies essentially what Polymer 3 elements are trying to accomplish in the npm ecosystem? Elements don't To me that's the element asserting that it's requesting a dependency that it does not have control of. It's expecting a sibling dependency and not a child dependency right? For all front-end polymer elements, yes their dependencies should be peer dependencies since the convention is to require by path up to a sibling folder. However, this doesn't mean that all dependencies in the node_modules tree have to be peer. We can still get the benefit of nested dependency trees for other modules that are not importing each other by path. Say for example, we have build tool that uses moment for dates, and our element is also using moment, but a different version. The packages would be build-tool package.json {
"name": "build-tool",
"dependencies": {
"moment": "^1.0.0"
}
} build-tool.js
my-element package.json {
"name": "my-element",
"peerDependencies": {
"@polymer/polymer": "^3.0.0",
"moment": "^2.0.0"
}
} my-element.js
The node_modules tree would be
Using yarn with |
Does peerDependencies actually enforce a same level install? It's certainly not documented like that: https://docs.npmjs.com/files/package.json#peerdependencies |
Npm doesn't enforce or install any peers since npm v3. All it does is issue a warning to developers that essentially says
It'll issue a warning nonstop each time you use an npm command until you install the missing dependency. The upside is that since they're installed at the top level by the app, they're ensured to always be siblings and resolve paths correctly to each other. The downside is that some elements may have a lot of peer dependencies (such as polymer, paper-styles, iron-behaviors, etc), and the user will have to This is a big downside, but I think it's a better step in the right direction of using the npm ecosystem in the way it was designed without introducing third-party tools (yarn) or radically different npm workflows (flat installs). We're early in the process, so the install hassle problem could be remedied with tooling or a new npm feature to auto-install peer dependencies. Something like |
Ahh right - you are required to have them as direct dependencies of your app. Manually installing all the dependencies is no worse (actually I view it as better) than maintaining resolutions with flat mode. |
Right, though that problem is way easier to fix than potential problems with flat mode and all the existing npm modules out there that would have issues with flat. The ability to auto-install peer dependencies wouldn't be hard to add to npm under a flag and completely alleviate the install pain. Npm could even generate a warning such as npm install --save @polymer/paper-button
npm ERR! peerinvalid The package @polymer/paper-button does not satisfy its siblings' peerDependencies requirements!
npm ERR! peerinvalid Peer @polymer/paper-button@3.0.0 wants @polymer/polymer@^3.0.0
npm ERR! Run npm install --peers to install missing dependencies.
npm install --peers As an interesting side note, peer dependencies are how Angular ensures only one version of their packages are installed as well as only one version of Rxjs and Zone.js |
hi~ we've got a thread going over at package-community/discussions#2 about this issue. For those who don't want to read through the Dostoyevsky book that thread's become, here's the tl;dr of what my team's planning on implementing:
tl;dr: $ npm i --save-asset @polymer/example
$ echo '<script type="module" src="/assets/@polymer/example/example.js"></script><example></example>' \
> index.html
$ npx serve
...(open http://localhost:5000/index.html and play with <example>) Note: I've popped into the polymer slack if anyone wants more back-and-forth questions, and we've also got conversations going on about this in the Discord for https://package.community. I probably won't say much more in this thread, specifically, but I figured this'd be useful. ❤️ |
I'm all for discussions on this type of front, but in the short term the |
@FredKSchott could you give an update about the status of |
@TimvdLippe I think we'll have an update on this shortly. |
@justinfagnani any update on this? I've opened Polymer/tools#403 where I'm proposing a different way for polymer-cli to resolve to a single package location that won't require use of yarn or a flat dep structure. Would love to get thoughts on that or at least know how the team is moving forward to remove this package's reliance on yarn's flat dep structure. |
fyi, not sure why commenting on this thread unassigned someone from it. Note: that happened automatically and by nothing specifically done on my part |
I fully understand the desire for flat dependencies and I think Polymer can be opinionated about requiring flat mode for apps. However, when a dependency has the
flat: true
in it's package.json, it forces any project which uses it into flat mode. This breaks all my existing projects.This parameter is already ignored by npm so effectively what this parameter does is prevent developers from using yarn unless they use it exactly how the Polymer Project intends. And since flat mode and separate client/server dep folders is extremely non-standard, the end result is this may very well fracture the development space and be every bit as divisive as bower was.
With
flat: true
specified in Polymer core or in any other dependency, my choices are:1 . Abandon yarn and go back to npm so that my projects can bypass the flat mode requirement.
2. Republish every dependency I use to an internal npm repository. This is a maintenance nightmare.
As I understand it the problems flat is attempting to solve are:
if
to work around this.I recommend documenting the flat mode requirement and having the Polymer provided build tooling check for it, but removing the
flat: true
property from any npm-published library intended to be used as a dependency.The text was updated successfully, but these errors were encountered: