Add "silent" option

[jescalan]

I really like that the polyfills were separated from the core in the latest release. Making a faster loading experience for users of modern browsers is for sure a priority over shipping a more bulky library for the benefit of a small percentage of users on legacy browsers.

I upgraded without adding polyfills, but wanted to take it even a step further -- if a user is on IE, I'd rather have the plugin not run at all than commit to loading polyfills into all browsers or muddying up the code with a runtime conditional polyfill load for IE. Since this plugin is not functional and is purely aesthetic for my use-case, my preference is just to have it not load for legacy browser users - I am using it for id-based jump links, so the browser native functionality still works and it will jump to the section, but without the smooth scroll.

To accomodate this, I added a new option, silent, which is false by default, but if set to true will suppress the error thrown when the browser doesn't support the features required by smooth-scroll. Setting this option to true indicates that you are aware that older browsers will not support smooth-scroll, but are OK with it.

Surely this feature can be dangerous is set without careful consideration of where and how smooth-scroll is being used, which is why it is false by default.

I tested this feature using IE11 and it is working as intended.

I also added a npm script to run the gulp compile, which makes it easier and quicker for contributors to run without having the correct version of gulp installed globally 😁

[cferdinandi]

Can you tell me a bit more about this?

I also added a npm script to run the gulp compile, which makes it easier and quicker for contributors to run without having the correct version of gulp installed globally 😁

[jescalan]

Oops, closed/reopened by accident!

Sure - db200f5#diff-b9cfc7f2cdf78a7f4b91a753d10865a2R37

If you clone down the project, install dependencies, and run npm run build, it will build out the project now with this "script" in place, automatically using the local version of gulp from the dependencies, and without any need to have gulp globally installed on your machine. Tiny change, but makes things a little more accessible 😁

[cferdinandi]

Ok, that's awesome!

Regarding silent errors: I actually think this is a great move. In looking at how other projects handle this, though, it seems like it's more common to handle this "automatically."

**As in, the unminified version is considered development and throws errors, while the minified versions are considered production and do not. **

That feels a bit more elegant to me, but I'm open to being persuaded otherwise. What do you think? I've already got a working version setup to support this on my machine.

[jescalan]

Hmm - interesting! A couple thoughts:

I feel like this change would be more likely to cause problems for users overall. It's one thing to note in the readme that the production version has no explicit compatibility checks, but it's another thing to actually have the production version run the compatibility checks and silently shut down if it's not compatible without giving any waring. I wouldn't be shocked at all if a torrent of "doesn't work in IE" issues started pouring in if this were implemented. As much as readmes are great, people only actually read them very rarely, unfortunately.

You could modify the proposal to say that the production version just doesn't check for browser support or compatibility at all to reduce extra code, but that would still result in errors - as soon as it hits a spot where one of the unsupported globals is used, there would be an error still thrown, but a more confusing one.

I'm also not aware of any major libraries that silently error by default, even in the production version. I know that many libraries give much more detailed error logs and traces in the dev than the production version, which I think is reasonable, but I don't think those same libraries actively suppress errors in production - if something goes wrong, you still get an error but perhaps not as thorough of an explanation to save some bytes.

I also don't feel like it's exactly safe to assume that the non-minified version is only going to be used in dev. With bundlers like webpack being so common, most people actually prefer to consume an unminified version, or better yet an ESM version from the package source, and let their own bundler setup do the minifying. In order to be safe with this assumption, I think you'd need to follow react's example and bundle separate dev and production versions explicitly, regardless of whether they are minified or not.

So I guess overall I don't really feel like that approach is the best direction, but of course it's ultimately up to you as the author of this package (and by the way, thank you for writing this! it has been incredibly useful for me and my employer 😁 )

[cferdinandi]

Not sure if you're aware, but the error message is actually a new addition. Prior versions all failed silently if supports() returned false.

I mention this because the change would actually be a reversion to the default for the .min versions, with the thrown error being an enhancement for unminified versions of the build.

My understanding was that React, Vue, and Angular all have production and dev versions, and the big differences in production were that:

1. They're minified
2. They don't throw as many errors

[cferdinandi]

Good catch on destroy(). Let me give some more thought to the silent vs. loud errors.

[jescalan]

Just checking in again on this one!

[cferdinandi]

I've decided not to move forward with this.

[jescalan]

Are you just closing and moving on, or have you decided to take a different approach? Just curious

[cferdinandi]

I was perhaps too hasty in closing this one. Thinking about this more, I like the your idea of providing user configurable options.

Reopening with intent to add an option to activate/silence errors.

[jescalan]

😅