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

Conversion du code vers ES2015 #63

Merged
merged 4 commits into from
Jan 19, 2017
Merged

Conversation

thom4parisot
Copy link
Owner

@thom4parisot thom4parisot commented Jan 18, 2017

fix #55

@thom4parisot thom4parisot requested a review from DirtyF January 18, 2017 15:27
@DirtyF
Copy link
Collaborator

DirtyF commented Jan 18, 2017

@oncletom grosse PR 😨

T'as converti avec un outil, t'as linté le code ? Faut tester que tous les codes fonctionnent ?

@thom4parisot
Copy link
Owner Author

J'ai converti à la main ;-) J'ai linté (travis le fait aussi).

J'ai testé quelques examples, pas tous. Je dirais qu'il faudrait juste essayer ceux qui paraissent "dangereux". Et je ferai un truc pour tester les examples à plus grande échelle plus tard (juste changer de version de Node ou une dépendance peut péter des trucs)

@gmarty
Copy link

gmarty commented Jan 18, 2017

T'aurais pu utiliser Lebab.

@DirtyF
Copy link
Collaborator

DirtyF commented Jan 18, 2017

@oncletom Souhaites tu effectuer une transformation auto ou pas du coup ?

Y'a t-il des règles à suivre ? (utiliser seulement let et les arrow functions ? lebab est assez souple à ce niveau et se configure.

'use strict';

var cycles = 0;

while(cycles++ < 1000000000);

module.exports = cycles;

Avec Lebab :

const cycles = 0;

while(cycles++ < 1000000000);

export default cycles;

On voit que le mode strict saute, const remplace var, et que l'export de module utilise la nouvelle syntaxe. Pas mal pour 3 lignes déjà, j'imagine sur des parties plus importantes ou plus complexes.

Si tu as transformé à la main, il y a surement une bonne raison et une transformation de syntaxe sera peut-être moins pédagogique dans certains cas.

@thom4parisot
Copy link
Owner Author

Merci @gmarty pour suggérer lebab ! J'en avais entendu parler mais pas lu en détail la doc. Chose faite désormais.

Y'a pas mal d'éléments de conversion qui sont manquants (d'après la doc) ou unsafe. Ça mange pas de pain de sélectionner des flags de conversion, de voir ce que ça donne dans une PR et de comparer les deux.

Dans le cas de ce livre, j'ai envie de garder l'export en module CommonJS pour que le code soit interprétable par Node.js nativement. C'est bon de savoir que le use strict; n'est plus nécessaire :-)

@SebastienElet
Copy link

Le use strict; semble toujours nécessaire si tu ne passes pas par babel (qui le rajoute en transpilant).

Je viens de tester en node 6.9.4 avec les examples de mdn : https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode/Transitioning_to_strict_mode#Differences_from_non-strict_to_strict

@DirtyF
Copy link
Collaborator

DirtyF commented Jan 19, 2017

@oncletom En ne sélectionnant que les options safe de lebab et en gardant strict et l'export de modules CommonJS, ton code est compliant 🎉

Les deux seules exceptions mineures :

chapter-02/examples/src/interrupt.js

--process.stdin.on('data', function(){});
++process.stdin.on('data', () => {});

chapter-03/examples/src/snippets/npm-cli-table.js

--table.push.apply(table, modules);
++table.push(...modules);

Ça c'est pour la syntaxe, maintenant faut encore tester avec node 6.9.4

Copy link
Collaborator

@DirtyF DirtyF left a comment

Choose a reason for hiding this comment

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

LGTM 👍 (syntaxement parlant)

@@ -1,19 +1,14 @@
'use strict';

var Users = require('../lib/models/user.js');
var argv = require('yargs')
const Users = require('../lib/models/user.js');
Copy link
Collaborator

Choose a reason for hiding this comment

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

FYI ce fichier n'est pas présent et génère une erreur lors de l'exécution en ligne de commande :

❯ node chapter-03/examples/src/snippets/cli-app.js
module.js:471
    throw err;
    ^

Error: Cannot find module '../lib/models/user.js'

@thom4parisot
Copy link
Owner Author

Merci @DirtyF ! Je pense que j'ai bourdé et soit jamais commité models/user.js soit je pensais l'écrire et je ne l'ai jamais fait. Je le case dans une issue à part 👍

@SebastienElet ah, merci pour le complément ! Ça va me servir pour #4.

@thom4parisot thom4parisot merged commit 27a9f48 into master Jan 19, 2017
@thom4parisot thom4parisot deleted the feature-es2015-syntax branch January 19, 2017 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Chapitre 2 - Mettre à jour les exemples avec la dernière version LTS ?
4 participants