devinekask / generator-devine-boilerplate

⚙ the one and only devine.be boilerplate generator
23 stars 3 forks source link

Yarn check, npm fallback #17

Closed thibmaek closed 7 years ago

thibmaek commented 7 years ago

Oplossing voor #15

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

thibmaek commented 7 years ago

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 commented 7 years ago

Ga het morgen eens bekijken. Zou voor https://github.com/devinehowest/generator-devine-project/issues/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 commented 7 years ago

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

wouterverweirder commented 7 years ago

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 commented 7 years ago

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

geoffreydhuyvetters commented 7 years ago

Kan je volgende zaken nog aanpassen:

Denk dat het dan wat leesbaarder gaat worden

wouterverweirder commented 7 years ago

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 commented 7 years ago

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

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

thibmaek commented 7 years ago

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 commented 7 years ago

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

wouterverweirder commented 7 years ago

Enkele voorbeelden van installs die nie lukken met yarn:

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 commented 7 years ago

Deze waarschijnlijk?

https://github.com/yarnpkg/yarn/issues/756

wouterverweirder commented 7 years ago

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

geoffreydhuyvetters commented 7 years ago

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

wouterverweirder commented 7 years ago

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 commented 7 years ago

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

wouterverweirder commented 7 years ago

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

geoffreydhuyvetters commented 7 years ago

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

geoffreydhuyvetters commented 7 years ago

ook cache clean gedaan

wouterverweirder commented 7 years ago

node versie? 6.8.0 hier.

geoffreydhuyvetters commented 7 years ago

ook

wouterverweirder commented 7 years ago

Raar. Op 7.0.0 werkt het wel. 6.8.0 niet.

geoffreydhuyvetters commented 7 years ago

ik draai 6.8.0, werkt ook bij mij

wouterverweirder commented 7 years ago

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 commented 7 years ago

jep.

screen

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

geoffreydhuyvetters commented 7 years ago

pty.js installeert ook perfect

geoffreydhuyvetters commented 7 years ago

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

wouterverweirder commented 7 years ago

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

geoffreydhuyvetters commented 7 years ago

zou dus gewoon voor die check gaan ipv flag

geoffreydhuyvetters commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

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