String attributes not being applied to script element

[dunncl15]

I'm noticing that string attributes (e.g. 'data-testid') are not added to the script element properly. In the example below I'm loading the google api script, but when I inspect the script element upon load the data-testid property is missing.

const [loading, error] = useScript({
    src: 'https://apis.google.com/js/api.js',
    type: 'text/javascript',
    'data-testid': 'gapi',
  });

I'd like purpose updating the code to use setAttribute(key, attributes[key] rather than scriptEl[key] = attributes[key]. I'm happy to open a PR for this if you're onboard with the change.

[hupe1980]

Do you mean this case?

react-script-hook/src/__tests__/use-script.test.tsx

Lines 32 to 55 in 1cdca7a

	it('should append a script tag with attributes', async () => {
	expect(document.querySelectorAll('script').length).toBe(0);
	const { result } = renderHook(() =>
	useScript({
	src: 'http://scriptsrc/',
	'data-test': 'test',
	async: true,
	}),
	);
	const [loading, error] = result.current;
	expect(loading).toBe(true);
	expect(error).toBeNull();
	const script = document.querySelector('script');
	expect(script).not.toBeNull();
	if (script) {
	expect(script.src).toEqual('http://scriptsrc/');
	expect(script['data-test']).toEqual('test');
	expect(script.async).toBe(true);
	}
	});

[dunncl15]

I do. I did run your tests locally and they seem to pass, however when implementing this hook elsewhere the 'data-test' key is not added as an attribute. Even just creating a dummy script tag and trying to add a 'data-test' key via the console doesn't seem to work. Using setAttribute instead of direct assignment did seem to work from what I've seen.

const scriptEl = document.createElement('script');
script.src = 'testscript.com';
script['data-test'] = 'test-id';
// scriptEl ==> <script src="testscript.com"></script>

[hupe1980]

Yes, then let's change that. Setting the attributes via the API is also a bit cleaner. Do you want to create a PR?

[dunncl15]

Yeah sure I'll open one shortly.

[dunncl15]

@hupe1980 I have a branch with these updates. Can you double check the access rights for the repo? I can't push currently.

[hupe1980]

You have to fork my repo, push your change and then create a pr

[hupe1980]

Merged #7 into master.

[lyallh]

Hi. Unfortunately PR #7 breaks the onload callback.

It seems setAttribute does not handle attributes like onload properly and the event handler doesn't fire.

I'll PR my hotfix to consider that sets only non-standard attributes with setAttribute.