Yarn check, npm fallback

[thibmaek]

Oplossing voor #15

Succesvol getest in 2 verschillende nvm installs (v7 incl yarn & v6 excl yarn)

[thibmaek]

Voor de check ipv which voor command -v gekozen omdat which geen exit code returnt bij not found. command -v zou in meeste shells moeten werken (getest in bash 4).

Er zitten wat ongerelateerde commits bij omdat ik afgebrancht heb van thibmaek/v3 die al wat werk bevat voor #10. Best squashen dus 😄

[geoffreydhuyvetters]

Ga het morgen eens bekijken. Zou voor #10 wel nog eerst even een soort leidraad willen voorzien. Had al wat zitten experimenten met http-client maar deed niet exact wat ik wou.

[thibmaek]

Same here voor http-client, daarmee dat ik mijn WIP ook nog niet als pr gestuurd heb. Ik wacht dan wel af :)

[wouterverweirder]

Alternatieve check die volgens mij cross-platform zou moeten werken:

const resolvePackageManager = () => {
  let packageManager = `npm`;
  try {
    require(`child_process`).execSync(`npm list -g yarn`, { encoding: `utf8` });
    packageManager = `yarn`;
  } catch (ex) {
    console.log(`yarn not available, using npm instead`);
  }
  return packageManager;
};

[thibmaek]

Is idd cleaner maar breekt wel als je yarn met brew geinstalllerd hebt of via shell script bv.

[geoffreydhuyvetters]

Kan je volgende zaken nog aanpassen:

• check sync ipv async in initialize fase yeoman
• prop yarn op true of false zetten
• prop gebruiken bij install fase (voor buildpack en npm/yarn install)
• package.json op npm houden

Denk dat het dan wat leesbaarder gaat worden

[wouterverweirder]

Eigenlijk kun je gewoon yarn --version testen en op basis daarvan beslissen:

const resolvePackageManager = () => {
  let packageManager = `npm`;
  try {
    require(`child_process`).execSync(`yarn --version`, { encoding: `utf8` });
    packageManager = `yarn`;
  } catch (ex) {
    console.log(`yarn not available, using npm instead`);
  }
  return packageManager;
};

[wouterverweirder]

Net gemerkt: Yarn heeft moeiten met het resolven van node_modules in install scripts. Is bijvoorbeeld een issue bij:

• modules die gebruik maken van node-gyp om C++ addons te compilen
• gebruik van electron-rebuild in een post-install script.

Yarn dus nog niet forceren maar optioneel houden. Eventueel meegeven als een CLI flag en niet automatisch voorkeur geven.

[thibmaek]

Werkt met laatste commit door yo devine-project <--yarn|-y> uit te voeren. Yeoman Constructor niet als ES6 method def kunnen schrijven om dat die blijkbaar niet constructable zijn:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Method_definitions#Method_definitions_are_not_constructable

[geoffreydhuyvetters]

@wouterverweirder hoe en wanneer ben je dit exact tegengekomen, ergens een issue op de yarn repo?

[wouterverweirder]

Enkele voorbeelden van installs die nie lukken met yarn:

• yarn add tty.js
• yarn add electron-rebuild

yarn kan node-gyp niet resolven (wat gebruikt wordt in een post install script) - nochtans zit die package in de dependency tree van die packages. npm doet dit wel correct.

Ik ben nog door de issues van yarn aan het scrollen, en zal een bug reporten.

[geoffreydhuyvetters]

Deze waarschijnlijk?

yarnpkg/yarn#756

[wouterverweirder]

@duivvv nee, is nog iets anders. Die issue gaat over yarn die native rebuilds overschrijft. Heb een issue aangemaakt yarnpkg/yarn#1710

[geoffreydhuyvetters]

net eens geprobeerd en heb geen miserie als ik electron-rebuild installeer via yarn

[wouterverweirder]

Vreemd. Laatste versie yarn, clean cache?

$ yarn cache clean
$ yarn add electron-rebuild
error /Users/wouter/MEGAsync/experiments/nodejs/yarn-tests/node_modules/nslog: Command failed.
Exit code: 1
Command: sh
Arguments: -c node-gyp rebuild
Directory: /Users/wouter/MEGAsync/experiments/nodejs/yarn-tests/node_modules/nslog
Output:
module.js:471
    throw err;
    ^

Error: Cannot find module '/Users/wouter/.yarn-config/global/node_modules/yarn/node_modules/node-gyp/bin/node-gyp.js'
    at Function.Module._resolveFilename (module.js:469:15)
    at Function.Module._load (module.js:417:25)
    at Module.runMain (module.js:604:10)
    at run (bootstrap_node.js:394:7)
    at startup (bootstrap_node.js:149:9)
    at bootstrap_node.js:509:3

[geoffreydhuyvetters]

ja bizar, werkt hier perfect. Heb ooit ook wel eens wat miserie met node-gyp gehad

[wouterverweirder]

Test je eens in een gewone shell, niet in die electron shell?

[geoffreydhuyvetters]

0.16.1 werkt in zowel Terminal als 'die electron shell' :)

[geoffreydhuyvetters]

ook cache clean gedaan

[wouterverweirder]

node versie? 6.8.0 hier.

[geoffreydhuyvetters]

ook

[wouterverweirder]

Raar. Op 7.0.0 werkt het wel. 6.8.0 niet.

[geoffreydhuyvetters]

ik draai 6.8.0, werkt ook bij mij

[wouterverweirder]

Doet ie de compile van de native module? Je zou in node_modules/nslog/build/Release map moeten hebben. Indien niet dan is de node-gyp niet uitgevoerd. Mogelijk ander error-logging niveau?

[geoffreydhuyvetters]

jep.

sorry, de bedoeling was dat die GIF hier kwam, had me van issue vergist.

[geoffreydhuyvetters]

pty.js installeert ook perfect

[geoffreydhuyvetters]

leek me al vreemd aangezien node-gyp ook in andere modules veel wordt gebruikt en er nog niks over te vinden was

[wouterverweirder]

node 6.8.0 ervan gezwierd en clean install gedaan, resolve issue is geresolved daarmee. Vreemd.

[geoffreydhuyvetters]

zou dus gewoon voor die check gaan ipv flag

[geoffreydhuyvetters]

heb een _spawn helper toegevoegd, en een yarn prop default op true gezet, voeg jij de check toe in initialize en test je het even @thibmaek

[thibmaek]

@duivvv Getest met latest commit en werkt op een node versie met yo en npm.
Output wel naar /dev/null gestuurd anders stond er een '/usr/bin/bash yarn not found'

[geoffreydhuyvetters]

bon, merged, heb een beetje zitten prutsen met de commits.. maar denk dat het ok is nu.