feat(InstantSearch): add in dev timeout when start is not called

[HeroProtagonist]

Summary

Closes #2671

When NODE_ENV === 'development' and this.started has not been set. A new timer will be set each time addWidgets is called. Only one timer is ever active and when this.started is called, it will be cleared.

Result

When widgets are added during development with yarn dev if .start() is not called within 5 seconds a warning will be displayed in the console.

[algobot]

Deploy preview for algolia-instantsearch ready!

Built with commit d2a93e6

https://deploy-preview-3238--algolia-instantsearch.netlify.com

[Haroenv]

Thanks a lot! This was the initial way I was thinking. @francoischalifour, do you have any idea how we can make this work with CDN? Maybe we should add a dev bundle 🤔 I think it would be worth it, so we can ship minified error messages too

[Haroenv]

NOTE: in v3 this will need to be changed to the logger function we added for consistency

[francoischalifour]

It's already available in v2: https://github.com/algolia/instantsearch.js/blob/develop/src/lib/utils.js#L426

@HeroProtagonist You can replace console.warn with this utility function :)

[HeroProtagonist]

@francoischalifour

In the constructor, console.warn is called a few times (I copied log format from there). Would you want me to add the utility function in those places as well?
https://github.com/algolia/instantsearch.js/blob/develop/src/lib/InstantSearch.js#L137

[francoischalifour]

Sure, feel free to update those in another PR!

[francoischalifour]

Thank you @HeroProtagonist!

@Haroenv I've got some plans on creating a development and production bundle. I'm not sure when this can fit in our roadmap though.

I think we should wait for these new bundles before adopting this new strategy and only include it in the development bundle.

[Haroenv]

Since this is actually a fairly low amount of code, and only a single timer, I think we could add this code as-is

[francoischalifour]

It would be the first time using process.env.NODE_ENV in our source code, meaning it's used only when running yarn dev for our contributors. IMO it would include too much potential complexity at a time where we are already overloaded.

Waiting to include this in the actual user bundle would provide much more value. What do you think?

[samouss]

Thanks for the work @HeroProtagonist!

But I agree with @francoischalifour I would prefer to wait until we have a proper support of development / production bundle too. We can tackle all those issues related to the DX once we have setup a proper build chain for development / production.

[HeroProtagonist]

I updated the pr to use warn. Not knowing enough about the project as a whole I will leave the future of this PR in your hands!

I did notice that no where else in the file process.env.NODE_ENV was used. I was thinking that with a bundler that has dead code elimination configured then those blocks would be ripped out. Because the blocks would turn into if (false) .... However, then the user would need to have that set up properly

[francoischalifour]

We will leave this PR on hold for now, waiting for better strategies on our side to handle development-only features.

Thank you for your work, this is greatly appreciated.

[francoischalifour]

Follow up on this PR – we introduced a way to execute code only in development enviroment with #3260, using the global __DEV__. The warn() function was renamed warning() in #3367 and its signature changed.

We however decided not to move forward with this development feature because we haven't really felt the pain of forgetting to call search.start() yet.

Thank you for your contribution though, it made us realize the need for development-only features earlier than originally planned.