bazaarvoice / scoutfile Goto Github PK

With the move of module code from this repository into bv-ui-core, the next step is to remove the stub modules we left in place.

This will be a breaking change for many clients, but seems like the right thing to do.

It will require some finessing of documentation in addition to code.

Instead we should be using .npmignore to avoid publishing node_modules of scoutfile.

https://docs.npmjs.com/misc/developers#keeping-files-out-of-your-package

As it is, users of this repo will have to manually check in scoutfile's node_modules.

The check as to whether an object is a DOM node has been found to fail under a specific condition where a site uses the loader module side by side with PrototypeJS (v1.7).

Prototype does some extending of the DOM interface, and is pretty non-intrusive in most of the browsers, so the instanceof Element check should work -- unless of course you are in IE8.

We recently added the check to this repo but I don't think this specific case was tested.

To verify that this check won't work with prototype, load up this page in IE8 and try document.getElementsByTagName('body')[0] instanceof Element

The problem

The loadStyleSheet method injects a link tag as a sibling of the first script node, which doesn't work on Firebird's container page because we replace everything within the body tag soon thereafter.

Proposed solution

Allow an option called injectionNode that lets us specify a different node (in our case the head). This lets us customize the behavior of loadStyleSheet without changing the behavior for anyone else.

Under some circumstances (large file or slow connection, e.g.) it may be normal for a script to take more than 1000ms to download. Should we increase the default to something like five or 10 seconds?

When using loadScript, I noticed that if my script takes longer than the default 1 sec, the script will continue to load and execute. Is this by design? In this scenario, onerror still gets called and eventually cleanup gets called too.

This happens in all browsers except for IE8 which relies on the script.onclick(); trick, which never gets called because onreadystatechange gets removed during cleanup.

Certain applications need to be able to set attributes on inserted script tags; the module should provide a mechanism for specifying attributes in the options passed to loader.loadScript().

63040d8 moved some dependencies from devDependencies to dependencies, which results in them being installed when we are installed as a dependency.

There is a line break on line 92 that breaks the link for 'Including common functionality via modules'

See #8. It is surprising that IE8 does not evaluate the script if onclick is not called. I would expect the loaded script to be evaluated, but the callback not invoked.

Hello,

I'm a little unfamiliar on this concept and I was wondering if you had any tutorials for setting this up with a BazaarVoice widget e.g. Spotlights?

I ran into problems when using loadScript to load the main JS file for Spotlights in IE8. In the main JS file, document didn't have the properties and methods that we needed. I noticed that these were undefined: document.documentElement, document.querySelector, and document.body. There are probably many others that are undefined.

To verify that this issue is with loadScript, I swapped it out to use https://www.npmjs.com/package/load-script instead, and IE8 was working fine.

We should decide on a minimum browser support level. I suggest IE9+.

It should be possible to specify a banner to be added to the top of the output file. Really it just needs to be a wrapper around http://webpack.github.io/docs/list-of-plugins.html#bannerplugin. We should support all the options supported by that plugin, especially the raw option.

I've noticed that sometimes webpack will fail but not log any errors to console. I believe it is something to do with the logic here

I've had to tinker with that conditional before, unfortunately I don't recall what I did to get the errors to display.

Try https://webpack.github.io/docs/cli.html#display-error-details.