-
Notifications
You must be signed in to change notification settings - Fork 13.8k
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
Polymer example has build step in it #1490
Comments
I also opened a question regarding this issue here: http://stackoverflow.com/questions/33296543/how-to-rebuild-polymers-todomvc-app |
@LoveAndHappiness As to rebuilding... in the readme checkout this section https://github.com/tastejs/todomvc/tree/master/examples/polymer#making-updates :) |
Vulcanizing a Polymer app is a best practice and we want to encourage folks to use this tool for performance reasons. TodoMVC is supposed to encourage that. While having a minified/built file is not the norm for these demos, it's an unfortunate necessity for us until HTML imports are native everywhere in browsers. Unfortunately, people also see these live demos and make FUD comparisons between frameworks. I want devs to know the RightWay to build an app. See my response https://groups.google.com/d/msg/polymer-dev/GocWEmUP8zE/VjtscPbTDQAJ on how to update/dev the todomvc setup. If you switch elements.build.html to elements.html in the index file, all is well :) |
We should close this issue. Updating the sample is already documented. |
As much as I love polymer and especially as much as I love your work and that of your team, I really feel that TodoMVC is here to encourage people to try these new libraries and frameworks so that devs, regardless of their skill level in the specific framework, can use TodoMVC as an introduction. Even the TodoMVC website says
A build step doesn't allow for editing the code to see how it feels, because there is the elements.build.js. I understand your point of view that an app is only ready, if it is vulcanized at this point in time. And I agree and encourage you to make it obvious in the docs. But I believe that the mission of TodoMVC is to give devs an understanding on how the app works, not necessarily how the app looks like when it is finished. And at least my approach has always been to clone the repository and to rebuild the app and try my hand at editing the code to see how it feels. And right now that's not possible, because it is vulcanized and I don't know how to unvulcanize it. I really hope I can change your mind on this matter, because I love polymer a lot and think it is a great piece of work and my recommendation would help devs get into the library easier. |
It's a two line change to remove usage of the vulcanized files :) If there's a way for todomvc.com to serve the built version and devs get the unbuilt version on checkout, that would make me happy. We could switch the default to elements.html instead of elements.vulcanized.html. But I'm not familiar with how the site deploy works. |
I have to agree with you (@ebidel). It's a two line change to remove it and I wasn't aware of it when I cloned the repo, because the first thing I did was to remove all css and all comments and changing the import afterwards didn't make any difference because I used an async property. Anyway, I will close the issue now. Though I suggest commenting the vulcanized to avoid future confusion. Thanks |
@ebidel @LoveAndHappiness going to reopen this for a moment, would love to hear @arthurvr's and @sindresorhus's thoughts around this because overall we try and avoid "compiled" code for the examples for the reasons you so elegantly pointed out @LoveAndHappiness
You can see this in place with our vanilla JS react examples and how we are using the browser version of the JSX compiler, which you would never do in the real world |
The goal of TodoMVC is meant to demonstrate app structure and design patterns. Neither is lost in serving a vulcanized version. Refer to #1490 (comment). The app structure is no different when using the vulcanized file. The key point is that the unminified source is checked in alongside the vulcanized bundle and it's easy to switch between for testing. |
I was against it from the beginning and don't see why this app needs to be different than all other build systems, which we also disallow. It adds complexity and doesn't add any value when comparing frameworks. It only makes it (even if it's just a little bit) more difficult to dig into the source code and the performance improvements don't really matter, we even warn people about that right in the readme:
|
We deliberate don't have strict rules for or against compiled code, because there's often a balance to strike between idiomatic use and "editability". Both are core goals with TodoMVC but I think it's best to make this decision on a case by case basis. As @arthurvr pointed out, loading times is not something we optimize for, but in Polymer's case it's more about the HTML imports than that. |
Agree with Pascal. Polymer is unique here in that it requires polyfills. No Loading is super fast in Chrome because it's native. Other browsers need On Sat, Oct 24, 2015, 3:54 AM Pascal Hartig [email protected]
|
I believe there has been plenty of opportunity to bring forward all arguments, so that the issue can be dealt with or closed, because we can expect that no additional information will be introduced. Polymer has a build step in it that is only retroactively alterable by going through the comments. There is consensus that this build step adds additonal complexity and also implies that developers also have to know how to implement the build step to reach the demonstrated application, when rebuilding the application. @arthurvr rightfully suggested that any other framework could also implement build steps, best practices and design patterns and thus obfuscate the end result. Thus making it necessary to scan through comments and readmes to understand how the application has been created. @ebidel claims that the reason for adding complexity through build steps is that developers would draw So the next thing to happen is to decide: Does the fear of drawing incorrect conclusions by developers as @ebidel claims outweight the need for a consistent and simple TodoMVC application? In my opinion no, because simplicity outweights perfectionism. Simplicity allows for a quicker understanding and faster adoption. I am rather new to github, so I am not sure how things are decided here. If it is democratic I pledge for an alteration of the code. |
I am in agreement with you @LoveAndHappiness Polymer having a build step makes it an outlier (of examples that can be run without a compile step {not compile-to-js}) in our examples, while yes it is not representative of a performant polymer app, the same can be said about almost all of the other examples, they are using techniques that are not the fastest since we are optimizing for simplicity and not performance. 👍 to drop the build step from polymer and go back to the raw version. Thanks for the thought out comment @LoveAndHappiness As a team, we are thinking around ways to give devs an option to build and deliver an "optimized" version but that conversation can happen in another issue. |
Please don't drop it. Polymer is unique to every example in TodoMVC because it requires polyfills. Am I right that no other example does?
@LoveAndHappiness it's far from an assumption. This is from years of working on this stuff and talking with developers. People are very quick to form FUD and draw incorrect conclusions about libraries without knowing detail or context. Added to this is the unfortunate fact that TodoMVC has become a "standard" for benchmarking libraries/frameworks. But that's where we've ended up.
I'd argue that anyone interested in the Polymer example should also be interested in making fast apps. For the short term, this requires them to need vulcanize. TodoMVC is a great way to discover TheRightWay to build an app in a lib/framework. Not incorporating a best practice into a beginner's example, is IMO, irresponsible. The Polymer team wants to start developers down the right path. This is really important and something all samples in TodoMVC should really adopt. People turn to these examples for patterns and best practices.
Given that people are learning from these examples, anyone checking out code will be scanning though readmes and looking at comments. I don't think that should be a concern. My proposal still stands: |
As a project, we've increasingly seen TodoMVC used for performance benchmarking, despite not being originally designed for this purpose. Our primary goals have always been ease of code readability and learning, as reflected by comments from core above. That said, the onus ultimately falls on us to craft a very good solution for the perf use-case if we want to continue to be used for this purpose. Safari/WebKit, Chrome, and FilamentGroup are all folks that have used us for this over the last 2-3 years and I think we have value to offer there. Right now, almost none of our apps are prod ready at all. I see a few possible paths moving forward (there are likely many more):
Ultimately, if we go for any of these options I think including a /ducks |
The polymer app seems to have a build step that concatenates (maybe vulcanizes?) every module into one giant build.elements.html along with a giant build.elements.js file.
As a newcomer it isn't clear how the app is actually build and structured, when the repository is cloned, compared to other examples, like angularjs or vuejs, where the structure is pretty clear.
I recommend removing the build step from the polymer example, so that newcommers to polymer are able to rebuild the todoapp, since it is posted for eductational purposes.
The text was updated successfully, but these errors were encountered: