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

Read defaults from environment #218

Open
dy opened this issue Sep 21, 2017 · 13 comments
Open

Read defaults from environment #218

dy opened this issue Sep 21, 2017 · 13 comments

Comments

@dy
Copy link

dy commented Sep 21, 2017

Just a user-point complaint.

If I have a repo with /index.html for github-pages in the base folder (say regl-line2d), then running budo test.js (a very easy and natural workflow) starts serving this index.html instead of expected test.js. I understand there are reasons for serving that index.html (which btw?), but is there something wrong with the simple workflow I have? I don't want to delete index.html and rebuild it every git push, neither I want to have gh-pages branch or a separate directory for gh-pages, as it is difficult (sorry yes it is awful laziness).

Same issue is with bankai.

Thanks.

@mattdesl
Copy link
Owner

mattdesl commented Sep 21, 2017 via email

@dy
Copy link
Author

dy commented Sep 21, 2017

@mattdesl oh yes what's that secret option? Sounds perfectly what I need :) Is there a way to enable this option by default? I think I may just modify budo pkg code globally.

@mattdesl
Copy link
Owner

Should be --forceDefaultIndex.

I’m still not sure it’s necessary. I’ll take a look at your repo tomorrow since I believe the “remapping” feature might solve your issue?

@dy
Copy link
Author

dy commented Sep 21, 2017

@mattdesl thanks, that is exactly --forceDefaultIndex. Tbh I don't get how remapping should help - I just buildindex.html for gh-pages once in a while and rest of the time work as budo test, not really caring about htmls. So this option is perfect!

If there is no way to enable this option by default, I think the issue can be closed then.

@dy
Copy link
Author

dy commented Sep 21, 2017

Something like reading these defaults from environment, with process.env should be as simple as

module.exports.defaults = {
  forceDefaultIndex: !!process.env.BUDO_FORCE_DEFAULT_INDEX,
  title: 'budo',
  port: 9966,
  debug: true,
  stream: true,
  errorHandler: true,
  portfind: true
}

@dy dy changed the title Ignore index.html in the current directory Read defaults from environment Sep 21, 2017
@dy
Copy link
Author

dy commented Sep 21, 2017

@mattdesl I can come up with a PR if you like

@mattdesl
Copy link
Owner

mattdesl commented Sep 27, 2017

Just looked at your repo a bit more, your build script is interesting and something I haven't encountered before. In most cases I don't embed my JS bundle in the index.html as it causes a worse user experience (the site waits until finishing download before it can render anything).

It seems odd to include an environment variable for this edge case, and I don't want to go down the route of including an environment variable for each flag. We've had issues in the past with env vars, and I think using a flag would be cleaner for your repository's users as well.

So your development script could look something like:

"scripts": {
  "test": "budo test.js --live --force-default-index -- -g bubleify",
  "build": "browserify test.js -g bubleify | indexhtmlify | metadataify | github-cornerify > index.html"
}

Alternatively, if you want, you could build the JavaScript to a separate file, and leave index.html as a simple HTML file that points to a script tag. This is where the "remapping" feature becomes handy.

See here for an example:
https://github.com/mattdesl/browserify-example/blob/2e0e66c243cd6cbbb7f6dcac40b6562d03a6e766/package.json#L21-L22

Maybe indexhtmlify could even provide a flag like --script bundle.js which generates a <script src="bundle.js"> instead of always expecting stdin. Then you could generate the HTML in the npm script as well.

@dy
Copy link
Author

dy commented Mar 13, 2018

Recently updated budo and run against this issue once again.

It seems odd to include an environment variable for this edge case

That is odd, yes, but it is so much more pleasant to run budo test (as good as npm test) in a folder with index.html, rather than npm run test-browser (since package may have node test script). Sadly that seems to be only my workflow.

@dy
Copy link
Author

dy commented May 16, 2018

Prob @etpinard would vote for this feature too.

@mattdesl
Copy link
Owner

I am still hesitant to add this feature, it would be odd to add it only for certain flags, and I do not want to add it for every single flag. Adding it just for some specific flags may make your life easier but it can introduce more complexity for other users, as well as an additional maintenance burden.

@dy
Copy link
Author

dy commented May 16, 2018

Some not elegant solutions

  • shortening flag to -fdi
  • monkey-patching budo globally every update
  • using package.json field similar to browserify with default budo settings
  • using .budoconfig, in a way similar to .htaccess since this is a web-server
  • having a wrapper package
  • prepare script dialog asking about how budo is used (browserify or dev server)

Philosophically that seems to be a question of whether budo a browserify-server or a web-server.

@mattdesl
Copy link
Owner

mattdesl commented May 16, 2018

How about a shorthand of -f? I really don't like adding more configuration files (users already have enough; budo is supposed to be easy and zero config).

In my own work I use a fork of module-generator which allows me to not have to re-write package.json fields on each module. You could also do this if you are always getting tired of adding budo test.js manually to scripts.

Also, you can use npx to trigger the local version of budo from within a project, instead of adding an npm script.

So it could look like this after the shorthand patch:

npx budo index.js -f

(This is not really recommended; it's better to have it in npm scripts so that others cloning your repo will just need to run npm t).

@dy
Copy link
Author

dy commented May 16, 2018

+1 for budo -f test.js

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

No branches or pull requests

2 participants