Skip to content
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

L'implémentation de Promise (ES6) écrase celle existante #67

Closed
sylvainpolletvillard opened this issue Nov 3, 2016 · 6 comments
Closed
Assignees

Comments

@sylvainpolletvillard
Copy link
Contributor

sylvainpolletvillard commented Nov 3, 2016

Bonjour,

Actuellement, toutes les extensions sont accompagnées d'une implémentation des Promise ES6 (a priori issue de https://raw.githubusercontent.com/jakearchibald/es6-promise/) qui vient écraser l'implémentation existante, même si elle est gérée nativement par le navigateur.

Cela cause divers soucis, notamment lorsque celle-ci est monkey-patchée comme c'est le cas sur les applications Angular 2.

Il faudrait tester si le navigateur implémente ou non les Promise, avant de forcer cette implémentation. Idéalement, charger le polyfill uniquement lorsqu'aucune implémentation n'est trouvée, afin de réduire un peu la taille du bundle javascript.

Je n'ai pas encore trouvé à quel endroit des sources et à quel étape du build cette implémentation était rajoutée. Si je trouve d'ici là, je proposerai une pull request.

@sylvainpolletvillard
Copy link
Contributor Author

https://github.com/IGNF/geoportal-access-lib/blob/master/lib/external/promise.js#L943

C'est cette ligne là qui fait le test d'une implémentation existante. Ce test est un peu trop précis dans la mesure où il vérifie que le nom du constructeur est bien Promise, alors que l'implémentation des Promise selon Angular2 (les ZoneAwarePromise) ne prennent pas cette peine. Cela dit les deux implémentations sont fiables.

Je vais remonter le problème sur le repository du polyfill. Ça prendra sans doute quelques semaines et il vous faudra mettre à jour la version de cette dépendance externe ensuite 😞

Ou alors si vous voulez me dépanner rapidement, vous pouvez remplacer cette condition par if (P && P.resolve() instanceof P && !P.cast) { . Le test sera tout aussi fiable et ne viendra pas remplacer les implémentations existantes, même non standard.

@gcebelieu
Copy link
Member

Bonjour,

Lorqu'on n'est pas en mode AMD, Le bundle est constitué une fois pour toute. On ne peut pas l'adapter à la volée selon le navigateur qui le charge.

On peut par contre effectivement essayer d'être plus fin dans le test de l'implémentation du Polyfill.

@lowzonenose : tu pourras te pencher sur le sujet ?

@lowzonenose
Copy link
Contributor

lowzonenose commented Feb 24, 2017

stefanpenner/es6-promise#230
stefanpenner/es6-promise#224
stefanpenner/es6-promise#165

Incompatible avec AMD !?
stefanpenner/es6-promise#263

Nouvelle version en release :
https://github.com/stefanpenner/es6-promise/releases/tag/v4.1.0

@gcebelieu
Copy link
Member

Parfait, il ne nous reste plus qu'à embarquer cette nouvelle release.

@lowzonenose
Copy link
Contributor

Une avancée !
angular/zone.js#499

@sylvainpolletvillard
Copy link
Contributor Author

Il leur aura seulement fallu six mois ;) Merci pour le suivi du ticket, la prochaine version de zone.js embarquera donc mon patch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants