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

Add npm init step to npm installation #141

Merged
merged 2 commits into from
Oct 17, 2018
Merged

Add npm init step to npm installation #141

merged 2 commits into from
Oct 17, 2018

Conversation

stevewillson
Copy link
Contributor

Add instructions to the npm installation section. Create a new folder for slides and then do npm init in that folder. If slides are stored away from the directory where 'node_modules' exists, then there will be an error when attempting to generate slides from a file not found issue.

Add instructions to the npm installation section. Create a new folder for slides and then do npm init in that folder. If slides are stored away from the directory where 'node_modules' exists, then there will be an error when attempting to generate slides for a file not found issue.
@obilodeau
Copy link
Member

Hi @stevewillson. Thanks for your suggestion!

Right now, I'm not sure of the benefit of having users going through the npm init questionnaire just so they are able to render some slides in a different directory. Plus, I don't see why they wouldn't have a file not found issue unless they install their package globally.

Can you please provide some more insights with maybe current output of commands and what you expect your pull request will fix? Thanks

@mojavelinux
Copy link
Member

It does seem that since npm v5, the install command will fail if a package.json file is not already present. It will succeed but report errors if only the node_modules directory is present.

The simplest way to fix is to add this command to the start:

$ test -f package.json || echo {} > package.json

Another option is to force yes on all questions when running the init command:

$ npm init -y

I prefer the first because I don't agree that the defaults for npm init are good.

For what it's worth, the following command succeeds without error when run in an empty directory.

$ yarn add asciidoctor.js@1.5.5-3

@stevewillson
Copy link
Contributor Author

stevewillson commented Sep 26, 2017

I am new to using npm and followed the current steps listed in the README.adoc file.

Install node.js

$ npm i --save asciidoctor.js@1.5.5-3
$ npm i --save asciidoctor-reveal.js

This stored the packages in my user's home directory (/home/user). I store the directory where I want to generate slides in another directory within my user's home directory (/home/user/slides).

I attempt to generate slides by executing a 'generate_slides.js' file.

generate_slides.js:

// Load asciidoctor.js and asciidoctor-reveal.js                                
var asciidoctor = require('asciidoctor.js')();                                  
var Asciidoctor = asciidoctor.Asciidoctor();                                    
                                                                                
require('asciidoctor-reveal.js');                                               
                                                                                
// Convert the document 'presentation.adoc' using the reveal.js converter          
var attributes = 'revealjsdir=node_modules/reveal.js@';                                                                                               
var options = {safe: 'safe', backend: 'revealjs', attributes: attributes};         
                                                                                
Asciidoctor.convertFile('pres.adoc', options); 

I create a file called 'pres.adoc' and attempt to generate the .html file:

$ nodejs ./generate_slides.js

After trying to generate the slides, I receive the following error:

user@comp:~/slides$ nodejs ./generate_slides.js 
/home/user/node_modules/asciidoctor.js/dist/npm/asciidoctor-core.min.js:52
c.$$p=(g=function(c){var e;null==c&&(c=m);if((e=c["$handles?"](a))===m||null==e||e.$$is_boolean&&1!=e)return m;b.ret(c)},g.$$s=this,g.$$arity=1,g),c).
call(e);return this.$raise("Could not find a converter to handle transform: "+a)}catch(f){if(f===b.returner)return f.$v;throw f;}},M.$$arity=1),m)&&"f
ind_converter"})(c.get("Converter"),c.get("Converter").$$scope.get("Base"))}(b.base)};y.modules["asciidoctor/converter/html5"]=function(b){function m(
b,a){return"number"===typeof b&&"number"===typeof a?
                                                                                                                                                      
                                                                                                                        ^

Error: ENOENT: no such file or directory, open 'node_modules/asciidoctor-reveal.js/templates/jade/document.jade'
    at Object.fs.openSync (fs.js:653:18)
    at Object.fs.readFileSync (fs.js:554:33)
    at /home/user/node_modules/asciidoctor.js/dist/npm/asciidoctor-core.min.js:27:380
    at singleton_class_alloc.H [as $read] (/home/user/node_modules/asciidoctor.js/dist/npm/asciidoctor-core.min.js:28:456)
    at q.x [as $try_read] (/home/user/node_modules/asciidoctor-template.js/dist/main.min.js:10:454)
    at l (/home/user/node_modules/asciidoctor-template.js/dist/main.min.js:10:108)
    at Object.Opal.yield1 (/home/user/node_modules/opal-runtime/src/opal.js:1198:14)
    at Array.ːeach [as $each] (/home/user/node_modules/opal-runtime/src/opal.js:11240:26)
    at q.b [as $resolve_template] (/home/user/node_modules/asciidoctor-template.js/dist/main.min.js:10:314)
    at q.f [as $handles?] (/home/user/node_modules/asciidoctor-template.js/dist/main.min.js:9:260)

I was able to generate the slides by using:

$ cd /home/user/slides
$ npm init (accept all default values) 
$ npm i --save asciidoctor.js@1.5.5-3
$ npm i --save asciidoctor-reveal.js

I am also able to generate the slides by moving the generate_slides.js and pres.adoc files into the /home/user folder.

I realize that the default values for npm init may not always be best and a cleaner npm command is probably available.

Let me know if you need any more information to re-create the error.

@mojavelinux
Copy link
Member

mojavelinux commented Sep 26, 2017 via email

@obilodeau
Copy link
Member

Thanks @stevewillson for the full context.

@mojavelinux is it possible that they already reverted that behavior? I run npm 5.3.0. I've checked the changelog quickly and didn't see anything regarding node_modules destination path.

What version of npm do you guys run?

@mojavelinux
Copy link
Member

I tried with both npm 5.3.0 and 5.4.2, same results. npm add does not function correctly if the directory is empty.

@obilodeau
Copy link
Member

@mojavelinux I can't reproduce your npm issue...

$ npm --version
5.3.0
olivier@lafindumonde:~/src/asciidoc$ rm -r test-npm/
olivier@lafindumonde:~/src/asciidoc$ mkdir test-npm
olivier@lafindumonde:~/src/asciidoc$ cd test-npm/
olivier@lafindumonde:~/src/asciidoc/test-npm$ ls -la
total 8
drwxr-xr-x  2 olivier olivier 4096 Sep 27 09:58 .
drwxrwxr-x 28 olivier olivier 4096 Sep 27 09:58 ..
olivier@lafindumonde:~/src/asciidoc/test-npm$ npm i --save asciidoctor.js@1.5.5-3
npm WARN saveError ENOENT: no such file or directory, open '/home/olivier/src/asciidoc/test-npm/package.json'
npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN enoent ENOENT: no such file or directory, open '/home/olivier/src/asciidoc/test-npm/package.json'
npm WARN test-npm No description
npm WARN test-npm No repository field.
npm WARN test-npm No README data
npm WARN test-npm No license field.

+ asciidoctor.js@1.5.5-3
added 13 packages in 1.853s
olivier@lafindumonde:~/src/asciidoc/test-npm$ npm i --save asciidoctor-reveal.js
npm WARN deprecated jade@1.11.0: Jade has been renamed to pug, please install the latest version of pug instead of jade
npm WARN deprecated transformers@2.1.0: Deprecated, use jstransformer
npm WARN deprecated minimatch@0.3.0: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue

> bufferutil@1.2.1 install /home/olivier/src/asciidoc/test-npm/node_modules/bufferutil
> node-gyp rebuild

make: Entering directory '/home/olivier/src/asciidoc/test-npm/node_modules/bufferutil/build'
  CXX(target) Release/obj.target/bufferutil/src/bufferutil.o
../src/bufferutil.cc: In static member function ‘static Nan::NAN_METHOD_RETURN_TYPE BufferUtil::Mask(Nan::NAN_METHOD_ARGS_TYPE)’:
../src/bufferutil.cc:102:39: warning: this statement may fall through [-Wimplicit-fallthrough=]
       case 3: *((unsigned char*)to+2) = *((unsigned char*)from+2) ^ *((unsigned char*)mask+2);
               ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[...]
make: Leaving directory '/home/olivier/src/asciidoc/test-npm/node_modules/utf-8-validate/build'
npm WARN saveError ENOENT: no such file or directory, open '/home/olivier/src/asciidoc/test-npm/package.json'
npm WARN enoent ENOENT: no such file or directory, open '/home/olivier/src/asciidoc/test-npm/package.json'
npm WARN test-npm No description
npm WARN test-npm No repository field.
npm WARN test-npm No README data
npm WARN test-npm No license field.

+ asciidoctor-reveal.js@1.0.3
added 157 packages in 11.171s
olivier@lafindumonde:~/src/asciidoc/test-npm$ ls -la
total 56
drwxr-xr-x   3 olivier olivier  4096 Sep 27 09:59 .
drwxrwxr-x  28 olivier olivier  4096 Sep 27 09:58 ..
drwxr-xr-x 146 olivier olivier  4096 Sep 27 09:59 node_modules
-rw-r--r--   1 olivier olivier 42474 Sep 27 09:59 package-lock.json

If there are platform discrepancies then we should definitely file an upstream bug.

@mojavelinux
Copy link
Member

The command doesn't fail. But it does show the following error:

saveError ENOENT: no such file or directory, open '<cwd>/package.json'

That's why I suggest adding npm init, which will seed the package.json file. Why wouldn't we want to do that?

(I installed Node using nvm and I'm now using npm 6.4.1).

Btw, yarn doesn't have this problem, which is why I still prefer it.

@obilodeau
Copy link
Member

I just don't like the npm init questionnaire. I think it gets in the way. When you prepare a presentation, you don't want to think about a version number, a license or a repository...

Here's the UX with npm 6.4.1 and (node v10.11.0):

With npm init

$ npm init
This utility will walk you through creating a package.json file.
It only covers the most common items, and tries to guess sensible defaults.

See `npm help json` for definitive documentation on these fields
and exactly what they do.

Use `npm install <pkg>` afterwards to install a package and
save it as a dependency in the package.json file.

Press ^C at any time to quit.
package name: (test-npm) my slides
Sorry, name can only contain URL-friendly characters.
package name: (test-npm) slides
version: (1.0.0) 
description: 
entry point: (ad.js) 
test command: 
git repository: 
keywords: 
author: 
license: (ISC) 
About to write to /home/olivier/src/asciidoc/test-npm/package.json:

{
  "name": "slides",
  "version": "1.0.0",
  "description": "",
  "main": "ad.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC"
}


Is this OK? (yes) 
$ npm i --save asciidoctor-reveal.js
npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN slides@1.0.0 No description
npm WARN slides@1.0.0 No repository field.

+ asciidoctor-reveal.js@1.1.3
added 14 packages from 6 contributors and audited 17 packages in 1.066s
found 0 vulnerabilities

$ node ad.js 
$ xdg-open presentation.html

Several questions to answer. None useful to the task at hand (creating slides). Reminds me of the CPAN package manager with his questions about do you prefer ftp, gopher or http...

Without npm init stage

$ npm i --save asciidoctor-reveal.js
npm WARN saveError ENOENT: no such file or directory, open '/home/olivier/src/asciidoc/test-npm/package.json'
npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN enoent ENOENT: no such file or directory, open '/home/olivier/src/asciidoc/test-npm/package.json'
npm WARN test-npm No description
npm WARN test-npm No repository field.
npm WARN test-npm No README data
npm WARN test-npm No license field.

+ asciidoctor-reveal.js@1.1.3
added 14 packages from 6 contributors and audited 17 packages in 2.955s
found 0 vulnerabilities

$ node ad.js 
$ xdg-open presentation.html 

I just don't see why npm forces down the fact that whenever you have dependencies it is a "project" and it needs to have a package.json with all that info in it...

Bottomline: I prefer the UX w/o npm init, if you disagree, I'm ok with adding the step. I would like yarn if it fixes that problem but I have other priorities before looking at that (blog post w/ new release).

@mojavelinux
Copy link
Member

We're in total agreement that the questionnaire is ridiculous. And your comment verified that we're seeing the same results. So now we're in a place where we can advance the conversation.

The question we need to ask ourselves is, what's the bare minimum that is needed in package.json to make npm happy? As it turns out, it's really simple.

echo {} > package.json

With that in place, npm no longer complains. What do you think about adding this command in place of npm init?

@obilodeau
Copy link
Member

obilodeau commented Oct 16, 2018 via email

@obilodeau
Copy link
Member

I have done it here while resolving the conflict instead. Merging as soon as Travis tests passes.

@obilodeau obilodeau merged commit c7ada42 into asciidoctor:master Oct 17, 2018
@stevewillson stevewillson deleted the clarify_npm_installation branch October 19, 2018 00:40
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.

None yet

3 participants