ergo / polymer-ui-router

UI-router wrapper for Web Components
Apache License 2.0
22 stars 6 forks source link

Polymer 2 #3

Closed deltaepsilon closed 7 years ago

deltaepsilon commented 7 years ago

I can't figure out how to make a pull request to a new branch. This should really be a new branch, but I'm creating the pull request to master.

I saw your original work on this and wanted to port it to Polymer 2.0 for a new project that I'm developing. I loved uirouter on Angular 1.x, and I'm loving it in Polymer as well.

I had to start from scratch and copy/paste from your earlier work as necessary. In doing so I found some improvements. For instance, instead of creating a separate vendors/ folder and editing angular-uirouter.js, I was able to import the original angular-uirouter.js and create a mock angular that suppresses the errors.

I was also able to switch to using for uisref. It makes the code a lot easier.

Anyway, let me know what you think. I'm going to keep working on my fork as my project progresses.

ergo commented 7 years ago

I'm a bit surprised with your changes, my branch already works with 2.0 (I tested the demo pages - the demos require updates, but actual elements worked fine).

You also changed behavior of some elements, for example I wanted them to work without shadow DOM support - for better styling support without requiring to mixins and shared-styles.

I noticed you added a known issue to the readme, ill try to fix this in next release.

I don't know at which point you forked but right now we have a merge conflict so even without my reservations about the changes I have no easy way of incorporating. Can you try the latest version of the element and describe what it is missing from your perspective?

ergo commented 7 years ago

Also, does moving code to /lib solve some problem? I have never seen this pattern used by polymer elements.

deltaepsilon commented 7 years ago

This is an incompatible fork. I did it on a new branch, and I wouldn't want to mix it with your master branch.

Your master branch works with the 1.0 to 2.0 compatibility layer. I was trying to do a 2.0-only version, which cause all sorts of breakages.

You're right that the core elements don't need to be in /lib. I moved them there early on when I was still trying to maintain compatibility with your 1.0 implementation. I was trying to do a minimal fork to make it compatible with Polymer 2.0 without the compatibility layer. The original element files all imported /polymer.html, which doesn't exist in 2.0. My thought was to move the elements into /lib, delete the /polymer.html imports, and then do the imports at the top-level. I gave up on that, but forgot to move the files out of /lib :)

I forked a couple of weeks ago, before you made your recent edits to solve the uirouter-sref issues. I solved those issues using <slot>, and since 2.0 is all about Shadow DOM and I'm already using shared-styles, there was no loss for my use case.

My thought was that my work could live on a different branch of your project. It's not compatible with your master branch, but it doesn't really belong on my own repo, because I used your original concept and copy/pasted a bunch of your methods. I don't want to be that jerk that copy/pastes someone's code and passes it off as their own.

ergo commented 7 years ago

I see two ways to resolve this in a nice way. I would like at least for now (few months) to maintain comaptibility for polymer 1/2.x and see if the API and functionality is stable. After this time we can then migrate to pure 2.x component.

So if you want to help here I would love to collaborate on what I have right now and then later tag new incompatible release.

If you want to maintain your own fork I'm fine with that and I'm sure Chris would be too, after all this is what open source is about - as long as you keep the licenses for the code everything will be good. Long term I think it would be better to avoid effort duplication. Let me know your thoughts, either here or via Polymer slack channel.

ergo commented 6 years ago

Hey @deltaepsilon, as I've quite ok with current state of UI-router for polymer 1.x and there weren't any complaints in this area, I'm treating it as stable release. Right now I've started work on Polymer 2.x - the master branch should already work the same as stable. I need to do some ES6 cleanups and get rid of the Polymer.dom() calls. I thought you might be interested to know about this.

deltaepsilon commented 6 years ago

@ergo That's fantastic! I've been using my fork with Polymer 2 for a while now, but I've only been using a limited set of features. I have no idea of the other feature ported over correctly, so I'll be happy to switch to a better, more maintained version :)

ergo commented 6 years ago

@deltaepsilon Cool, and I've love to get some help if you encounter any issues with it. One big notable change is that UIView can pass one way bindings downwards to stomped elements.