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.

Sign up for GitHub

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

Jump to bottom

[polymer-cli] Consider allowing resolution to single package in polymer.json #403

Closed
markcellus opened this issue May 21, 2018 · 7 comments · May be fixed by gcloud-lerralice/tools#25
Closed

[polymer-cli] Consider allowing resolution to single package in polymer.json #403

markcellus opened this issue May 21, 2018 · 7 comments · May be fixed by gcloud-lerralice/tools#25
Labels
wontfix

Comments

@markcellus
Copy link

markcellus commented May 21, 2018 •
edited
Loading

I've brought this issue up about Polymer 3 quite a few times in the Polymer slack channel, but I'd like it to get more visibility...

Problem

There is a problem (identified here and here) where if you have the same dep multiple times in your dep tree that define custom elements, you will run into the following error when serving or building your Polymer project using polymer-cli:

Uncaught DOMException: Failed to execute 'define' on 'CustomElementRegistry': this name has already been used with this registry at Polymer

This problem can be solved by simply having a flat dependency structure. And for that, there is yarn, which ensures only one dep exists in the dep tree at any given time, but using flat: true with yarn breaks projects that contain deps that rely on npm's nested tree structure.

Proposed Solution

Let me first say that this is a problem that is pretty difficult to solve, so I definitely understand if the Polymer team may not want to dedicate its resources to a solution at this time. But what does the team think about adding the ability to specify, for each offending package, the path to the package in the dep tree the polymer app should resolve to in its polymer.json file? And ignore all other instances of the same package lower in the dep tree?

so in order to do

import {html, PolymerElement} from "polymer";
import "iron-pages";
import "@polymer/app-route";

the polymer.json file would contain the following...

{
  "package_prefix": "/node_modules",
  "packages": {
    "@polymer/polymer": "@polymer/polymer", 
    "iron-pages": {
      "main": "iron-pages.js",
      "path": "@polymer/iron-pages"
    },
    "@polymer/app-route": "@polymer/app-route/app-route.d.ts"
  }
}

NOTE: the "@polymer/polymer": "@polymer/polymer", explicitly instructs polymer-cli to always use the polymer package at root of node_modules and ignore any other instances of this package like in node_modules/@polymer/iron-pages/node_modules/@polymer/polyer, since the @polymer/iron-pages package also uses @polymer/polymer as its dependency.

I understand that this may not be an issue that Polymer has created because its just using the dep tree that is produced by npm (or yarn in some cases), but I do believe that this is an issue that would be nice for Polymer to address in its build process to ensure the ease and intuitiveness of the package. Also it makes for easier adoption of Polymer with very little understanding.
@markcellus markcellus changed the title Consider not breaking app when duplicate CustomElement packages exist in dependency tree [polymer-cli] Consider not breaking app when duplicate CustomElement packages exist in dependency tree May 21, 2018
@markcellus markcellus mentioned this issue May 21, 2018
Closed
@markcellus markcellus changed the title [polymer-cli] Consider not breaking app when duplicate CustomElement packages exist in dependency tree [polymer-cli] Consider allowing resolution to single package in polymer.json May 21, 2018
@coreyfarrell
Copy link
Contributor

I've posted an update to my babel plugin support forcing use of the root node_modules when resolving sources from specific packages. I've posted a pull request to my own repository, hoping that anyone who might be interested in this would comment on the API (plugin config keys). cfware/babel-plugin-bare-import-rewrite#1 or you can see the tree at https://github.com/cfware/babel-plugin-bare-import-rewrite/tree/enhancements

I realize this doesn't help polymer-tools itself but I needed something that was easier to use directly from babel. Hopefully this can inspire improvements to polymer-analyze / polymer-build.

@bahrus
Copy link

bahrus commented May 23, 2018

It seems that this results in the error this name has already been used with this registry:

<head>
   ...
<script src="https://unpkg.com/@polymer/paper-input@3.0.0-pre.19/paper-input?module" type="module"></script>
<script src="https://unpkg.com/@polymer/paper-button@3.0.0-pre.19/paper-button?module" type="module"></script>
</head>

Not sure if that syntax is right.

@markcellus markcellus mentioned this issue May 25, 2018
Closed
@btopro
Copy link
Contributor

btopro commented Nov 21, 2018 •
edited
Loading

less then ideal but this helps me use a mono-repo in local development via polymer serve https://www.npmjs.com/package/@lrnwebcomponents/deduping-fix

https://github.com/elmsln/lrnwebcomponents/blob/master/elements/deduping-fix/deduping-fix.js

@markcellus
Copy link
Author

@btopro that code surfaces which component is duplicated, which can be helpful. But it doesn't solve the original issue. I would hate to see the warnings when this issue pops up for a ton of elements 😬 (which is usually the case when dealing with non-flat dependency trees).

@btopro
Copy link
Contributor

btopro commented Nov 22, 2018 •
edited
Loading

oh ya, beyond less then ideal but a fallback :)
Here's what I've got currently to help avoid overlap. Not perfect as flat but thus far seems to work --

https://github.com/elmsln/lrnwebcomponents/blob/master/package.json#L27-L28
pre - https://github.com/elmsln/lrnwebcomponents/blob/master/scripts/preinstall.sh
post - https://github.com/elmsln/lrnwebcomponents/blob/master/scripts/postinstall.sh

super specific to the structure of ours but it works. I'd like to clean it up as a gulp task w/ lerna ls --json

@stale
Copy link

stale bot commented Mar 4, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Mar 4, 2020
@snyk-bot snyk-bot mentioned this issue Mar 7, 2020
Open
@snyk-bot snyk-bot mentioned this issue Jun 19, 2021
Open
@stale
Copy link

stale bot commented Apr 29, 2022

This issue has been automatically closed after being marked stale. If you're still facing this problem with the above solution, please comment and we'll reopen!

@stale stale bot closed this as completed Apr 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
wontfix
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Snyk] Fix for 1 vulnerabilities gcloud-lerralice/tools
4 participants
@btopro @coreyfarrell @markcellus @bahrus