Buslowicz / twc

TypeScript based, boilerplate-less, Polymer toolbox friendly Polymer Modules
32 stars 1 forks source link

HTML imports with bower inconsistencies #113

Closed mlisook closed 6 years ago

mlisook commented 6 years ago

Using the bower: syntax, if .bowerrc is not present, <link rel="import" href=" ... "> is generated with a path of ../../ regardless of the custom element's location in the project. If .bowerrc is present, the relative path is correctly calculated. But, in either case, on Windows, the path is generated with backslashes instead of forward slashes.

The bower: reference is also inconsistent with referencing another custom element in the project. To reference my-element.html one must use ./my-element not ./my-element.html (which would generate a link with href my-element.html.html), but with the bower: the .html is required.

tpluscode commented 6 years ago

Regarding your second point, I think it actually isn't inconsistent.

import './my-element`

is actually ES6 import syntax, which makes sense given that all two code are essentially modules.

The bower: imports on the other hand are a convention used to add dependencies to elements distributed as HTML Imports.

Buslowicz commented 6 years ago

Just as @tpluscode pointed out, twc uses ES6 imports, so no need to add .html, as relative imports of other twc modules will be .ts files anyway. Using ES imports means TS has no issues with hinting etc, so it's pure JavaScript/TypeScript flow. When those are compiled, paths will be converted to .html and everything will work.

As for incorrect paths calculation, could you provide some samples? Last time I checked it all worked fine, given the fact that paths are calculated to work with bower components flow (components end up being placed inside bower_components). To change this behavior, you need to add an environmental variable bowerDir or create the .bowerrc with the bower path inside of it, it was already asked in #102. If what you are talking about is indeed a problem with calculation, I will make sure to fix that, just need examples that break the current calculations.

As for backslashes instead of slashes, I got it reported twice already, will take care of that once I am back to the project (closer to the end of October). I'd love to do it quicker but I really can't :(. I can however give all the verbal help to fix that issue :). It is basically my bad of forgetting that front-end paths are platform agnostic and should not use path.sep, just plain /.

tpluscode commented 6 years ago

I'd love to do it quicker but I really can't :(

On the bright side browsers seem to handle the wrong slashes just fine :)

Buslowicz commented 6 years ago

Browsers fix a lot of developers mistakes, which is awesome, but is actually the cause why we have so bad quality of code all around the internet :P.

mlisook commented 6 years ago

I was certainly not trying to imply allowing or requiring .html on an import statement. If anything, I'm saying that bower imports probably should not require the .html.

tpluscode commented 6 years ago

That could work either way I think.

Maybe it will not be an issue when you combine two with potts. It generates module definitions for elements in bower_components. Still give you the .html suffix but at least you get IDE suggestions (and typings for element interfaces which is even cooler but beside the point).

Buslowicz commented 6 years ago

The extension in imports tells twc what is the thing you are importing. If you want to import an html module (like paper-button.html), you should use the correct name, as this is an html module. If there is a JS file you would import it with .js extension. The former translates to <link rel="import">, while the latter translates to <script> tag.

@tpluscode When I checked potts 2 days ago or so, I couldn't make it to work, could you confirm it works for you? If not, probably some dependency changed (maybe polymer-analyzer API) and I'll need to fix that too :/.

tpluscode commented 6 years ago

Yea it pretty much works. Well, breaks on non-Polymer elements (try nuclei/status-bar) but otherwise I haven't had issues.

Buslowicz commented 6 years ago

Probably due to it relying on polymer-analyzer :P. Anyway, will need to revise it at some point anyway.

mlisook commented 6 years ago

The path is correct, ../../ if you are writing an element to be distributed via bower, but not in a project, unless .bowerrc is present. So, if I have a structure like:

-project --bower_components --src ---my-great-element.ts

and I use bower: in my-great-element I'll get a relative path of ../../ instead of ../bower_components/

I can zip a minimal project and attach it.

Buslowicz commented 6 years ago

So yes, this is intended behavior. Most Polymer projects are single components so I made it a default. If you build a project, you might have bower_components, but you might change the name to libs or vendor or anything (using .bowerrc), so having that as a default might not target the larger audience. I will add an optional config file for twc which would allow this configuration as well, but for now the .bowerrc or bowerDir env var are required.

mlisook commented 6 years ago

OK, I can accept that, though possibly a tweak to the readme would make it more obvious. In any case, intended behavior is intended behavior. Thanks for your assistance.

Buslowicz commented 6 years ago

I will definitely put that in the readme, somewhere where it's hard omit. This is actually what people asked about a lot with polymer serve, so there will be a lot more questions about it. Nevertheless thanks for reporting ;).