Polymer / tools

Polymer Tools Monorepo
BSD 3-Clause "New" or "Revised" License
430 stars 200 forks source link

<!--! do not remove --> inserted by polymer build / polymer serve causes warning in IE11, and result in blank polymer-2-starter-kit #2465

Open LawrenceMok opened 7 years ago

LawrenceMok commented 7 years ago

what I did: polymer init select polymer-2-starter-kit polymer serve open the site in IE11, got a completely blank page, console shows these warnings:

HTML1414: Unexpected character: U+0021 EXCLAMATION MARK (!) 127.0.0.1:8081 (67,9) HTML1416: Unexpected character in comment end. Expected "-->". 127.0.0.1:8081 (67,10)

Also after around 10 seconds, got this error: SCRIPT445: Object doesn't support this action CustomElementInternals.js (255,7)

view source showing line 67 as: <!--! do not remove -->

which should be inserted by polymer build / polymer serve

colincannon-acp commented 7 years ago

Please help with this, as far as I can tell Polymer 2 does not work at all with ie11 after being built.

I removed the line

<!--! do not remove -->

After building and the original error was gone, but a different error popped up complaining about an unexpected end of file, and the page still did not display in the browser.

screen shot 2017-06-01 at 9 53 47 am

Also, I am getting the same error messages and problems in EDGE.

stramel commented 7 years ago

I was able to reproduce this as well. Although, I believe this issue would be better off being in the PSK repository.

Versions & Environment

Steps to Reproduce

  1. Install polymer-cli (`npm i -g polymer-cli)
  2. Create a directory for the app and change to that directory (mkdir test && cd test)
  3. Initialize polymer-2-starter-kit (polymer init polymer-2-starter-kit)
  4. Serve app (polymer serve -o)

Expected Results

App loads successfully in all supported browsers

Actual Results

App fails to load in IE11. Also, throws warnings in IE and Edge.

Browsers Affected

Console Output

Edge: image

IE11: image

FredKSchott commented 7 years ago

Hmm, okay we'll need to make time to dig in to why this is happening.

usergenic commented 7 years ago

Just updated bundler to preserve <!--! comments incidentally in https://github.com/Polymer/polymer-bundler/pull/546

ghost commented 7 years ago

Not even a quick workaround, just for the time being???

cgriffin4 commented 7 years ago

@andreabioli - I was able to get my page to work after a few hours of fiddling with things but unfortunately I don't remember exactly what I did. Sorry, and good luck.

I know I tried straight up editing the index file and couldn't get things working, tried using different versions of webcomponentsjs and didn't have any success. Then I uninstalled polymer-cli completely. Installed the newest version of polymer-bundler and then reinstalled polymer-cli. I think I also made some changes to the index file and my-app file but I'm not sure which of those changes were related to getting things working again.

As a side note, this kind of timeline on bugs which break Polymer-Cli on windows or IE/Edge really has me hesitant to ever upgrade after finding a working version and worried about the future use of polymer at my workplace.

ghost commented 7 years ago

...your last sentence is really scaring, but I think it's honestly objective. I got to love Polymer approach, clean and modern: we are developing a test project here where I work now, but very often I get puzzled by these things... It will be really a nightmare, if a different version of just one single component can break everything so easily... :-(

justinfagnani commented 7 years ago

I tried reproducing just the comment warning with the following jsbin in IE11, but couldn't: https://jsbin.com/kiditugizo/edit?html,output

And from reading this page: https://msdn.microsoft.com/en-us/library/hh180764(v=vs.85).aspx I don't think that warning should break the page. It might be two different things.

stramel commented 7 years ago

@justinfagnani I was noticing it today still on PSK#2.0-preview.

cgriffin4 commented 7 years ago

It will be really a nightmare, if a different version of just one single component can break everything so easily...

@andreabioli - It's not all that doom and gloom. For the most part versions between components work extremely well. The only issue I've seen has been updates/releases to new dev/build tools tend to have more bugs (sometimes crippling ones) in Windows OS/browsers. Polymer library and polymer components have been solid for the most part. That said, the tools are improving all the time and it's just generally not a good idea to immediately update production build tools so just take best practices and you'll be good.

For this issue it was certainly more than just the comment but it was how the build tool output the index file, specifically the section with the es5-custom-adapter.

That said, with the latest version of PSK (2.0-preview), version 1.2 of Polymer-Cli, and 1.5.1 of Polymer-Build I DO NOT see the problem any longer. @stramel - Could you provide versions or code where it is still an issue?

ghost commented 7 years ago

You are right, sorry for having a moment of bad mood, but in the afternoon I had another big problem just trying to use iron-validator-behavior (still in 'old mode') in Polymer 2: I filed the problem, and I already saw I'm not the only one... These things don't help to consider the whole approach in a very positive way, but I'm confident that you can progress through all of this! :-) Moreover, it's never a good think to criticize the (strong) efforts of other people, so sorry also for this!

rpapeters commented 7 years ago

Just checked with polymer-cli 1.2 on a Windows system and IE11; still having the same issue, so for me it is not resolved yet. (Also tried building using git bash). Sadly IE11 still must be supported so this is really a showstopper. Hope you are able to figure out what's going wrong, good luck!

Regards, Rob

Update: I did notice that the polymer-2-application (the simple template) DOES work in IE11.

cgriffin4 commented 7 years ago

@rpapeters I'd check your version of polymer-build and download a fresh psk preview 2. Hopefully you'll notice one of those has been updated and it'll narrow down where your issue is. Good luck

ghost commented 7 years ago

I made further tests. I see for example that only in the case of IE11 and Edge there is an incredible high CPU activity before serving the page (is it normal in IE and Edge, or is it only for Polymer applications? I don't know...), and then, beyond the warnings, that as someone else said, maybe don't have an impact on displaying the page, these are the errors I get:

SCRIPT438: Object doesn't support property or method 'call' patch-events.js (353,7) SCRIPT445: Object doesn't support this action CustomElementInternals.js (255,7)

That are probably the real responsible for NOT showing the page at all, in the end... To note that the second error always happens, the first only sometimes... just to add more strangeness...

bkawk commented 7 years ago

I too have the same issue you can see it at https://www.bkawk.com

screen shot 2017-06-27 at 8 01 11 am screen shot 2017-06-27 at 8 02 51 am
ghost commented 7 years ago

I can add even more: The class defined in CustomElementInternals.js is CustomElementRegistry, that at this page, https://developer.mozilla.org/en-US/docs/Web/API/CustomElementRegistry, is said to be NOT supported in Internet Explorer. I'm starting to suspect something... :-( Tracing the code, the problem is (probably) that in that class, the function _flush() is called with an internal variable (this._internals, line 142 of the file I have) that is undefined, and it's trying to call this._internals.patchAndUpgradeTree(document), that obviously can only fail...

ghost commented 7 years ago

Now I tried to jump over the offending line (CustomElementRegistry.js, line 142, this._internals.patchAndUpgradeTree(document);), and now I have no errors, only the warnings, but the page is totally blank, it looks like Polymer is not initialized at all...

ghost commented 7 years ago

...and if we run the validator on bkawk page: https://validator.w3.org/nu/?doc=https%3A%2F%2Fwww.bkawk.com We see a couple of minor errors, and then, this time NOT marked as a warning, an error regarding the famous/infamous line with the comment...

ghost commented 7 years ago

What worries me is this: nobody in the world can deliver at the moment a Polymer 2 application that works on IE??? Is there any update on this' High priority' bug? I'm really worried, now, because we have to go in production before the end of the month, and the reference browser is just IE! :-( Sorry for asking...

rpapeters commented 7 years ago

I feel your pain. We had to convince the customer to use an alternative browser while IE is their standard. Luckily for us they where prepared to do so, but this is NOT a good selling point... In a business perspective, the Polymer team should be on top of this, because (I think) many companies are still using (the crappy) IE browser. Although I really love building Polymer (2) apps and strongly believe it is the right way to go, I can't say it should be marked as production ready untill the IE problem is fixed. So, go go Polymer team! Keep up the good work!

Regards, Rob Peters

ghost commented 7 years ago

For me it's just a no point: I pushed to use Polymer, in this application, that will be run all over the world. Big numbers, in this, I cannot ask such a big company to make such a move (change browser!) just because of my (small) application... :-(((

HughxDev commented 7 years ago

I agree with @cgriffin4 and @andreabioli. This issue is blocking an app launch for my client, which is costing me time/money/reputation. And yeah, I saved IE testing till the end, which was a misstep, but Polymer says it supports IE11+, so I was overly confident that any IE-specific issues would be minor; not something like the entire app failing to load. I can’t even get things to work by side-stepping the cli with a custom gulp build that basically just does crisper and babel. It looks like I’m going to have to spend time downgrading the app to Polymer 1.9, since a previous 1.x project loads fine in IE. This honestly makes me reconsider using the library in production environments. I would expect this kind of error from a beta, but not a 2.0 release.

ghost commented 7 years ago

Just the same steps I did, @hguiney! :-) I'm still telling the customer 'don't worry, test it in Chrome, and when we go in production we will have the compatibility', but now I must prepare to downgrade to Polymer 1.x... :-( Really not a good thing... :-(

ghost commented 7 years ago

I had some hopes when I saw there was a new version of Polymer CLI. Unfortunately, I could quickly realize that even with Polymer CLI 1.3.0 this problem is still totally there... :-(

ghost commented 7 years ago

I FOUND THE PROBLEM!!! After a lot of research, I saw that the 'Shop' Polymer 2 Application is working in IE, so I starting 'disassembling' that application, inserting my own code (and the needed code from what was generated by the starter kit (or CLI, whatever... you know...) In the end it came out that in the my-app custom element the app-route is defined as follows:

    <app-route
        route="{{route}}"
        pattern="[[rootPattern]]:page"
        data="{{routeData}}"
        tail="{{subroute}}"></app-route>

Where the rootPattern variable is initialized like this:

      constructor() {
        super();

        // Get root pattern for app-route, for more info about `rootPath` see:
        // https://www.polymer-project.org/2.0/docs/upgrade#urls-in-templates
        this.rootPattern = (new URL(this.rootPath)).pathname;
      }

But the URL object is not compatible with IE (11), as we can see from this page:

https://caniuse.com/#search=url

And it blows up everything, in a very strange way, that's not easy to spot. So now I changed the route as in:

<app-route route="{{route}}" pattern="/:page" data="{{routeData}}" tail="{{subroute}}"></app-route>

Without the variable at all, of course removing the offending line! And it works without any problem, in any browser...

Do I deserve a prize, for one full day of tests? :-)

P.S.: now I'm curious... Please, could the other users check to see if this solve the problem?

ghost commented 7 years ago

And in this official page: https://developer.mozilla.org/en-US/docs/Web/API/URL/URL They explicitly say that 'This is an experimental technology', and even in this page it's stated that this is NOT supported in IE11. Using this class in a generic and global code generator is really a little bit crazy! :-))))))))))))))

krumware commented 7 years ago

related issue: https://github.com/PolymerElements/polymer-starter-kit/issues/1020

I'm a bystander in this one, but I'm guessing the version of the PSK which is pulled by the current polymer-cli is not the most up-to-date

jsilvermist commented 7 years ago

@andreabioli This is already fixed in the latest PSK branches, and will probably be released as soon as lazy-imports are finalized and implemented in the PSK.

A URL polyfill used to be shipped with webcomponents.js v0 but was removed in the v1 version, hence the confusion and issue with IE11.

ghost commented 7 years ago

I perfectly understand that errors happen, of course, anytime, anywhere... in this case, whatever the reason, you have to admit that it's really strange, that a whole team released such a giant error, and noone in testing realized this... Anyway, apart from the full day of craziness I spent on this, I'm happy that in the end I could spot it! Only this! :-) P.S.: by the way, I couldn't find the #1020 mentioned by @krumware :-( Someone could post the reference here! :-)

stramel commented 7 years ago

@jsilvermist working on getting documentation, perf profiling, and some fixes into the repo before we merge and tag.

Also, I think this should be closed since it IE11 and Edge just warn about the comment. Using the RegExp should fix the failing load on IE11.

rpapeters commented 7 years ago

Just updated polymer-cli to 1.3.1 and generated a fresh polymer-2-starter-kit app. Still seeing the same issue: ie11_psk_error

schlm3 commented 7 years ago

Same here. Discovered this in 1.1.0 already and its still there in 1.3.1. However, I can not see any negative impact on our polymer-1 application in IE except for the warning in the console. Of course it is pretty awkward, that google improves the polymer-linter to check for correct syntax and at the same time introduces illegal syntax during build...

ghost commented 7 years ago

Yes, Polymer 1... we were talking about the Polymer 2 starter kit, where you cannot even see the first page, screen in IE remains blank...

krumware commented 7 years ago

@unionthugface iron-ajax now requires the promise polyfill instead of including it. Check out the iron-ajax repo https://github.com/PolymerElements/iron-ajax/pull/280

moodyjmz commented 7 years ago

bower_components/shadycss/src/style-properties.js - I can see this being delivered as ES6 into IE11

And CustomElementInternals.js - which seems to be dynamically generated by polymer serve

This is cli v1.3.1

JoceM commented 7 years ago

Is there any updates about this ? This is pretty critical and strangely not widely discussed anywhere else. Folks reverting to Polymer 1 ?

stramel commented 7 years ago

This has been come a catch all issue and should be closed as the root issue has been addressed. The fix for the original issue is to switch from using new URL to a regex based lookup as pointed out here https://github.com/PolymerElements/polymer-starter-kit/issues/1020.

@FredKSchott @abdonrd @LawrenceMok Can we close this?

rpapeters commented 7 years ago

This fix is not yet available in the polymer-cli, or is it? If not, how could we already use this polymer-starter-kit fix inside polymer-cli?

stramel commented 7 years ago

polymer-cli master seems to be pulling the ^3.0.0 version of PSK. Which appears to not to have the latest tagged yet. So maybe this stays open until it is tagged? Follow status of tag here: https://github.com/PolymerElements/polymer-starter-kit/issues/1038

@abdonrd @justinfagnani

abdonrd commented 7 years ago

@stramel @rpapeters The problem is solved, but now we have to wait for a new PSK release. The Polymer CLI doesn't need to be updated, because it takes the last PSK tag.

rpapeters commented 7 years ago

Thx for the update, glad to hear the problem should be solved! Can’t we manually override the PSK tag that polymer-cli uses? Or manually install a different PSK version and let polymer-cli use that specific version?

abdonrd commented 7 years ago

@rpapeters you can download the PSK 2.0-preview branch.

rpapeters commented 7 years ago

Sorry for asking, which folder should the files go to, so polymer-cli wil make use of it? Somewhere into the node_modules?

abdonrd commented 7 years ago

@rpapeters No, I mean direct download:

https://github.com/PolymerElements/polymer-starter-kit/archive/2.0-preview.zip

Instead of:

polymer init polymer-2-starter-kit

rpapeters commented 7 years ago

Ok, clear. Thx!

Update: 'polymer serve' is working! 'polymer build --preset es5-bundled' is not working... image

Update2: 'polymer build --js-compile --js-minify --css-minify --bundle --add-service-worker' IS WORKING! I think --html-minify is breaking the build at the moment, because when I add that parameter it is not working.

motss commented 7 years ago

No luck in getting it serve without any issue on IE11. Edge was fine.

abdonrd commented 7 years ago

Released Polymer Starter Kit v3.1.0! 😄

stramel commented 7 years ago

Hooray!

rpapeters commented 7 years ago

Awesome!

phun-ky commented 7 years ago

Still getting this issue in IE11 11.540.15063.0