Skip to content
This repository was archived by the owner on Sep 20, 2019. It is now read-only.

Bundle custom-elements-es5-adapter #741

Merged
merged 1 commit into from
Mar 21, 2017
Merged

Bundle custom-elements-es5-adapter #741

merged 1 commit into from
Mar 21, 2017

Conversation

valdrinkoshi
Copy link
Contributor

Fixes #737 by providing a new bundle custom-elements-es5-adapter.js, to be loaded by the user before declaring new Custom Elements

@azakus FYI

@dfreedm dfreedm self-assigned this Mar 21, 2017
dfreedm
dfreedm approved these changes Mar 21, 2017
View reviewed changes
Copy link
Contributor

@dfreedm dfreedm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

notwaldorf
notwaldorf reviewed Mar 21, 2017
View reviewed changes
README.md Outdated
**TL; DR: ** Use `webcomponents-es5-loader.js` only if you want to deploy ES5 code to _all_ browsers (both with and without native custom elements support) -- if this isn't a requirement, then use
`webcomponents-loader.js` instead.
```html
<script src="bower_components/webcomponentsjs/custom-elements-es5-adapter.js"></script>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't you still need a loader for the actual polyfill?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes you do, this example was for Chrome. I shall put the loader in here i guess..!

@dfreedm dfreedm mentioned this pull request Mar 21, 2017
Closed
@notwaldorf
Copy link
Contributor

LGTM 2

@valdrinkoshi valdrinkoshi merged commit 5c801be into master Mar 21, 2017
@valdrinkoshi valdrinkoshi deleted the bundle-ce-es5-only branch March 21, 2017 21:13
@dfreedm dfreedm mentioned this pull request Mar 22, 2017
Closed
@nstepien
Copy link

@valdrinkoshi
Might be a good idea to compile the custom-elements-es5-adapter.js to ES5. Right now I'm seeing arrow functions.

@valdrinkoshi
Copy link
Contributor Author

@MayhemYDG the adapter MUST stay ES6 mainly because it has to extend HTMLElement class which doesn't have a constructor but can just be extended. See more details here https://github.com/webcomponents/custom-elements/blob/master/src/native-shim.js#L12

From my tests, in IE11/Edge loading custom-elements-es5-adapter.js doesn't cause any syntax error to be fired.

@nstepien
Copy link

Fair enough.

I do see a syntax error on IE11 though. 🤔
image

@valdrinkoshi
Copy link
Contributor Author

Oh weird. I've tested with the VM provided in modern.ie, with Windows 8.1 and IE11.
I would in fact expect it to throw a syntax error. Is there a way you can catch that error, e.g. onerror="" ?

@007lva
Copy link

007lva commented Sep 7, 2017 •
edited
Loading

@valdrinkoshi I've tested with IE11 on a Windows 7 VM and is clear that it doesn't support arrow functions.

@robdodson
Copy link

My (rough) understanding is you'll get a syntax error in IE11 but it's harmless so long as the adapter is not bundled with the rest of your code.

@robdodson
Copy link

actually i may be wrong, cc @justinfagnani to double check me on that :D

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@notwaldorf notwaldorf notwaldorf left review comments

@dfreedm dfreedm dfreedm approved these changes

Assignees

@dfreedm dfreedm

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@valdrinkoshi @notwaldorf @nstepien @007lva @robdodson @dfreedm