Skip to content

Conversation

@kurmasz
Copy link

@kurmasz kurmasz commented Aug 20, 2024

This PR only modifies one line in loader.js to failover to globalThis if this is undefined.

This fixes the problem of showdown crashing when used on the client side as a module (See https://github.com/kurmasz/showdown_mre) for a MRE.

@qlty-cloud-legacy
Copy link

Code Climate has analyzed commit 03aa099 and detected 0 issues on this pull request.

View more on Code Climate.

@KeithHenry
Copy link

This stops it crashing when ESM loaded, but it still doesn't load as a module and it still pollutes the global scope.

Your fix allows this code to run:

// import './showdown.esm.js'; // sync load as side effect 
// or
await import('./showdown_with_fix.js');

// Oops, this is actually window.showdown
const converter = new showdown.Converter();

In ESM terms this has loaded as a side effect - we'd want it to download as the module. This rules it out for most ESM libraries (which have to avoid doing this). It also creates issues running this in other contexts (such as a web worker).

To fix #660 this code should work:

// import * as module from './showdown.esm.js'';
// or
const module = await import('./showdown.esm.js');

const converter = new module.showdown.Converter();

// And critically:
if(showdown)
    console.error('Global scope polluted', showdown);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants