napcs/node-livereload

exts option should replace the defaults

Closed this issue ยท 13 comments

v0.6.2

As mentioned after #89 was closed, an array of extensions passed with the exts option gets appended to the default list instead of replacing it as you would expect. If you specifically don't want to watch one of the default extensions (ex: js), there's no way to override it currently.

napcs commented

How should this be implemented? Let's discuss before a PR.

I see 3 possible use cases for the exts setting. Right now, only 1 and 2 are possible.

  1. You want to use the defaults as provided.
  2. You want to add to the defaults.
  3. You want to completely replace the defaults.

Changing exts to replace would be a breaking change for current instances of case 2. It would still be possible to do it, but you would have to fully specify the default list, or maybe reference a provided static property. (ex: livereload.createServer({exts: livereload.defaultExts.concat(['foo'])}))

A backwards-compatible alternative would be to introduce another property, either an array to override the defaults or a flag to switch the handling of exts from concat to replace.

One possibility โ€“ instead of an array, pass a function:

exts: arr => arr.filter(x => x !== 'css'),
napcs commented

The more I think about this, the more I think it's best to break backwards compatibility with this option and implement a new option for the old behavior. exts will overwrite, and we'll have a new flag to append options.

I'll release LiveReload 0.6.3 with a noisy deprecation warning and 0.6.4 will remove the old behavior.

@napcs
If you intend to make this breaking change you should really release it as 0.7.0.

Or why not 1.0.0? :)

napcs commented

Yea really great point. We'll go to 0.7.0 for the next release.

Hey, thanks very much for your work on this.

I just wanted to point out that your deprecation notice really confused me.

*** DEPRECATION WARNING *** The exts option will REPLACE extensions in 0.6.4. ***

I thought exts was replacing an old option called extensions but wasn't clear why I was getting the warning. Then I thought maybe there is a by missing and exts will be replaced. Anyway, maybe I am dumb but it confused me. I would suggest:

*** DEPRECATION WARNING *** The exts option will REPLACE default extensions in 0.6.4. ***

I'm still really confused. I currently have this:

const liveReloadServer = liveReload.createServer({
  exts: ['marko'],
});

What do I need to do to fix this deprecation? How do I get rid of the message?

napcs commented

@nahtnam Nothing. The deprecation message exists because the behavior will change in the next version. You will continue to get this message until the next release. See my message above - noisy deprecation warning for 0.6.3.

Wait, for the same behavior to apply, wouldnt my code have to be:

const liveReloadServer = liveReload.createServer({
  exts: ['html', 'css', 'js', 'png', 'gif', 'jpg', 'php', 'php5', 'py', 'rb', 'erb', 'coffee', 'marko'],
});

The default exts are:

defaultExts = [
  'html', 'css', 'js', 'png', 'gif', 'jpg',
  'php', 'php5', 'py', 'rb', 'erb', 'coffee'
]
napcs commented

@nahtnam in the current version (0.6.3), you would just add "marko". It would add "marko" to the existing default extensions. But this behavior is confusing. So in the next (unreleased) version, the exts option will REPLACE all the default extensions with only what you specify. But that's what the deprecation warning you're seeing is all about - When I release the new version, if you're using the exts option, you might all of a sudden not be watching the files you expect. So we'll add a new option to preserve this behavior.

Yeah sorry, I meant that the above code would be what you would need to have the same output with the new version, correct?

napcs commented

0.7.0 is now released which closes this issue.

The -e or --exts option now replaces all default extensions.
The -e or --extraExts option appends default extensions to what you specify. This is the old behavior.