Skip to content

Conversation

@greggman
Copy link
Contributor

honestly I think you only need aframe-master.js

Description:

include only files in dist in npm pacakge

Changes proposed:

  • include only files in dist in npm pacakge

see #3600

greggman added 2 commits May 24, 2018 18:32
honestly I think you only need aframe-master.js
do a squash merge
@ngokevin
Copy link
Member

Thanks! The site installs A-Frame through npm at different versions / branches for documentation. I think we just need to also allow docs/ in as well for now, which shouldn't be too large.

@greggman
Copy link
Contributor Author

I'm curious why do docs need to be installed with npm?

Does the aframe library reference the docs directly? Is there some place where it says if you want to read the docs to go to <currentfolder>/node_modules/aframe/docs?

npm's point is to install code libraries. Those packages then often get bundled into apps. Anything in those folders get included into those apps. For example if someone makes an electron app or a phonegap app and includes aframe those docs will then be embedded in their app.

It seems like if people want docs locally they should clone the repo. npm packages are not for docs they are for code. Generally docs should appear somewhere else.

Just trying to understand why docs should be included.

@ngokevin
Copy link
Member

We used npm as a medium to install and copy documentation versioned from the GitHub repo. We could rejigger it to perhaps fetch and download the ZIP instead, but for another day.

The Markdown files (just flat files) shouldn't be included in applications if they aren't required. So easiest to just leave docs/ for now, and continue to wipe all else from npm.

@greggman
Copy link
Contributor Author

The Markdown files (just flat files) shouldn't be included in applications if they aren't required.

Unfortunately that's not true. The app has no way of knowing what files are needed. Require is not enough to tell. Some libraries have files the code references either by loading directly (node, electron) or by XHR/Fetch (browser) whether it's images, fonts, svgs, icons etc so they always have to include everything included in whatever npm downloads.

@ngokevin
Copy link
Member

Will just live with a few Markdown files for now so we don't have to break the entire documentation system (or spend days rejiggering the build system and the bot). At least the examples are out. npm is messy, if there are libraries that suck in everything they fetch from npm, that's rough.

@ngokevin
Copy link
Member

Gonna merge for now and push a fix to continue to include docs, apologies if it isn't ideal yet!

@ngokevin ngokevin merged commit 4e900ff into aframevr:master May 30, 2018
@donmccurdy donmccurdy added this to the 0.9.0 milestone Aug 31, 2018
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.

3 participants