elringus / bootsharp

Compile C# solution into single-file ES module with auto-generated JavaScript bindings and type definitions
https://bootsharp.com
MIT License
669 stars 36 forks source link

Support ES module import in browser #53

Closed DanielHabenicht closed 10 months ago

DanielHabenicht commented 2 years ago

Hi again,

I tested the library with angular and it is running into an error:

universalModuleDefinition:1 Uncaught ReferenceError: global is not defined
    at universalModuleDefinition:1:1
    at Object.9424 (universalModuleDefinition:1:1)
    at __webpack_require__ (bootstrap:19:1)
    at Module.1896 (home.component.html:14:263)
    at __webpack_require__ (bootstrap:19:1)
    at Module.721 (universalModuleDefinition:1:1)
    at __webpack_require__ (bootstrap:19:1)
    at Module.23 (app.component.html:6:8)
    at __webpack_require__ (bootstrap:19:1)
    at Module.8835 (environment.ts:16:71)

Which is expected as the angular team came to the conclusion to not support global in their webpack build for broad compatibility. https://github.com/angular/angular-cli/issues/9827#issuecomment-369578814

I think this library should not rely on some other process to supply the variable. So going forward you could use:

Workaround is adding (window as any).global = window; to polyfill.ts

elringus commented 2 years ago

Is this about this line: https://github.com/Elringus/DotNetJS/blob/cad9a87854fd860203d25991a55bb3a68fa11229/DotNet/Packer/LibraryGenerator/LibraryTemplate.cs#L11 ?

It should only invoke in node environment, so global should be accessible.

elringus commented 2 years ago

If it's that line, can you please test if the following will work with angular:

factory(module.exports, global || window); 
DanielHabenicht commented 2 years ago

It does not work, because global is not defined (so it produces the same error). You have to check if it exists before:

// Get global shim as in https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/globalThis#search_for_the_global_across_environments
var getGlobal = function () {
  if (typeof self !== 'undefined') { return self; }
  if (typeof window !== 'undefined') { return window; }
  if (typeof global !== 'undefined') { return global; }
  throw new Error('unable to locate global object');
};

factory(module.exports, getGlobal()); 
elringus commented 2 years ago

Wouldn't global || window return window in case global is not defined?

elringus commented 2 years ago

Or should it be global ?? window.

DanielHabenicht commented 2 years ago

You can try it out in your own web browser, just open the Developer Tools and navigate to the console: image

It's kind of unsuspecting, because I would have said the same before I tried it out.

elringus commented 2 years ago

Do you have any idea how we can make an automated test for this without involving angular?

DanielHabenicht commented 2 years ago

Execute the module in a browser context. This could be a way: https://www.sitepoint.com/using-es-modules/#:~:text=Safari%2C%20Chrome%2C%20Firefox%20and%20Edge,Here's%20what%20they%20look%20like.&text=Simply%20add%20type%3D%22module%22,executing%20each%20module%20only%20once.

Otherwise I think there is no way around implementing it inside an example project. Which would also be a way to test it.

elringus commented 2 years ago

Wait, but it does already work in browser (there is a sample project). The line with global shouldn't be invoked in browser anyway.

DanielHabenicht commented 2 years ago

For webpack projects it is invoked because they are loaded as module (independent of later being executed in the browser). I think it would be the same for react or any other webframwork.

elringus commented 2 years ago

I'm currently using the library in react, no issues here.

DanielHabenicht commented 2 years ago

Might be that react is already polyfilling global?

DanielHabenicht commented 2 years ago

I tried it out with stencil now, other error:

index-c37bab2d.js:2903 TypeError: Cannot set properties of undefined (setting 'dotnet')
    at app-home.entry.js:10:21
    at app-home.entry.js:11:3 undefined

Happen with or without the getGlobal fix.

import { Component, h } from '@stencil/core';
import * as dotnet from '../../../../Project/bin/dotnet';

@Component({
  tag: 'app-home',
  styleUrl: 'app-home.css',
  shadow: true,
})
export class AppHome {
  componentDidLoad() {
    dotnet.boot();
  }
  render() {
     ...
  }
}
elringus commented 2 years ago

I think I was able to reproduce this in hello world sample with the browser module as you've suggested:

<!DOCTYPE html>

Open console (Ctrl+Shift+I) to see the output.

<script type="module">
    import * as dotnet from './Project/bin/dotnet.js'

    dotnet.HelloWorld.GetHostName = () => "Browser";

    window.onload = async function () {
        await dotnet.boot();
        const guestName = dotnet.HelloWorld.GetName();
        console.log(`Welcome, ${guestName}! Enjoy your global space.`);
    };

</script>

— but using the getGlobal() doesn't seem to help:

    var getGlobal = function () {
        if (typeof self !== 'undefined') { return self; }
        if (typeof window !== 'undefined') { return window; }
        if (typeof global !== 'undefined') { return global; }
        throw new Error('unable to locate global object');
    };
    if (typeof exports === 'object' && typeof exports.nodeName !== 'string')
        factory(module.exports, getGlobal());
    else factory(root.dotnet, root);
elringus commented 2 years ago

The error seem to come from webpackUniversalModuleDefinition; could it be caused by the webpack autogenerated UMD wrapper?

DanielHabenicht commented 2 years ago

I think this is related: https://github.com/umdjs/umd/issues/127 and https://github.com/umdjs/umd/issues/134

a comment suggests to switch to esm, but I don't now if this is compatible with VSCode related targets

DanielHabenicht commented 2 years ago

Seems like the only way around is publishing two js files. One for ES modules and one as UMD.

elringus commented 2 years ago

I've added browser-es sample for testing (not working right now): https://github.com/Elringus/DotNetJS/blob/main/Samples/HelloWorld/browser-es.html

Not sure what's the best approach to solve this (producing 2 libraries is not optimal, as it will require a lot of extra logic in C# packer). Guess the best workaround for now is polyfilling global on the consumer side. Will appreciate any suggestions.

elringus commented 2 years ago

Might be that react is already polyfilling global?

More likely it's just I'm bundling the react app with webpack, which statically links all imports. If I got it right, this issue only matters when you try to directly import dotnet in browser as ES module. You can still use it with any framework or inside any app and then bundle it.

DanielHabenicht commented 2 years ago

Angular also uses Webpack under the hood. But alright you already changed the title of the issue to reflect on the ES module issue. Appreciate your investiagation.

DanielHabenicht commented 2 years ago

Well React is using kind of UMD only: https://github.com/facebook/react/issues/10021

elringus commented 2 years ago

Maybe angular is bundled differently, without static linking the deps? I mean, if bundle already has all the imports resolved/linked internally at build time, it shouldn't matter which import mechanism each individual dependency uses when the bundle is imported at runtime, right?

DanielHabenicht commented 2 years ago

Well yes angular is already using esm modules. Would you be open to including the polyfill for getGlobal anyway?

elringus commented 2 years ago

Would you be open to including the polyfill for getGlobal anyway?

If it'll solve the issue, sure. I've already tried doing that actually, but it didn't work, because we also need to share that global with dotnet/blazor internal stuff. But maybe I've missed something.

DanielHabenicht commented 2 years ago

If it'll solve the issue, sure. I've already tried doing that actually, but it didn't work, because we also need to share that global with dotnet/blazor internal stuff. But maybe I've missed something.

It is at least solving the issue for angular, as angular seems to use a different way for importing the module.

For importing the module via type"module" we would have to set this to window conditionally in dotnet.js:

!(function (e, t) {
    "object" == typeof exports && "object" == typeof module
        ? (module.exports = t())
        : "function" == typeof define && define.amd
        ? define([], t)
        : "object" == typeof exports
        ? (exports.dotnet = t())
        : (e.dotnet = t());
})(this, () => // <== Here
...

Then it is available via window.global.dotnet

elringus commented 2 years ago

I've tried replacing this to window, but it returned Uncaught TypeError: dotnet.HelloWorld is undefined.

Regarding angular, do you mean it's not related with the browser-es sample? In this case we'd need a minimal repro (without angular) as a starting point. Ideally, an automated test or at least a sample for manual testing.

elringus commented 2 years ago

I'm also not against dropping UMD target if there is a way to make it work with both CommonJS (require) and ES (import) in all the environments. Other UMD scenarios (global script import in browsers and AMD) are not that common nowadays, afaik.

Aloento commented 2 years ago

In my browser env, no global is defined, only globalThis @DanielHabenicht And then I don't think it has anything to do with the framework you're using (angular), because no browser will have a global var. In the browser (Worker or not) just call self, you will get var self: Window/WorkerGlobalScope & typeof globalThis

Aloento commented 2 years ago

I'm also not against dropping UMD target if there is a way to make it work with both CommonJS (require) and ES (import) in all the environments. Other UMD scenarios (global script import in browsers and AMD) are not that common nowadays, afaik.

If I were to do it, I would just write it all as an ES module and then polyfill it (like Babel) if needed.

elringus commented 2 years ago

If I were to do it, I would just write it all as an ES module and then polyfill it (like Babel) if needed.

The thing is we have autogenerated mono wasm JS wrapper and then our own runtime module. Both are bundled with webpack on top of which C# packer emits UMD initialization template. As it currently stands, we have to first figure how to configure webpack to produce a module that can be imported as both ES and CommonJS in all the environments and then update the packer initialization template.

Aloento commented 2 years ago

I haven't had a chance to read your code yet, but in my experience, it can be broken down into two steps. 1, generate JS (ES6) code by template 2, depending on the configuration to let the packaging tool (Webpack) handle it (e.g. polyfill and minimization)

For the second step, I would rather suggest you to use Rollup, which is ready to use with its polyfill and other functions. I will read your code as soon as possible.

elringus commented 2 years ago

depending on the configuration

We have to produce a single library, that should support both ES and CommonJS scenarios and all the environments (node, browser, vscode, etc).

Aloento commented 2 years ago

You should allow the user to define a configuration in the C# csproj file, such as

<JSFormat> 
  esm / amd
</JSFormat>

UMD is too big and generates a lot of unnecessary code, as well as ESM would be better if the user is using a newer environment. You don't need to support all environments at the same time, because that doesn't happen. Users should be allowed to make their own adjustments if they need to.

elringus commented 2 years ago

UMD templates are not big at all, literally about 10 lines: https://github.com/umdjs/umd/blob/master/templates

I would like to support all scenarios with a single library, if possible. After all, it's already working, except this one particular case with ES in browser.

Aloento commented 2 years ago

ESM modules have been supported since NodeJS 12. And common packaging tools (eg webpack) can escape cjs modules. Even Blazor officially supports import writing: https://docs.microsoft.com/aspnet/core/blazor/javascript-interoperability/call-javascript-from-dotnet?view=aspnetcore-6.0#javascript-isolation-in-javascript-modules

require is an obsolete thing and even warns in some packaging tools (UMI), and Preact's WMR does not even support it. So really, just pick one.

elringus commented 2 years ago

Right, but it's required in some environments, eg vscode web extensions won't work otherwise.

Aloento commented 2 years ago

Right, but it's required in some environments, eg vscode web extensions won't work otherwise.

So like I said, you generate JS with the latest write-ups and then let the user choose what to do with them. You don't have to consider any compatibility in the original output, your target can even be ESNext. Rollup (or webpack) then performs all the work needed to meet the user's needs. (eg. converting original output (ESM) to CommonJS then VSCode can use them)

elringus commented 2 years ago

In that sense it's already working like this. You can consume current library, post-process it with a bundler (webpack/rollup) and it will work anywhere. This issue is about importing the library as ES module directly in the browser w/o any post-processing.

Aloento commented 2 years ago

To check env, just:

const isBrowser =
  typeof window !== "undefined" && typeof window.document !== "undefined";

const isNode =
  typeof process !== "undefined" &&
  process.versions != null &&
  process.versions.node != null;

const isWebWorker =
  typeof self === "object" &&
  self.constructor &&
  self.constructor.name === "DedicatedWorkerGlobalScope";

const isJsDom =
  (typeof window !== "undefined" && window.name === "nodejs") ||
  (typeof navigator !== "undefined" &&
    (navigator.userAgent.includes("Node.js") ||
      navigator.userAgent.includes("jsdom")));

const isDeno = typeof Deno !== "undefined" && typeof Deno.core !== "undefined";

Try to decide which factory and global should be called based on the above code.

DanielHabenicht commented 2 years ago

The "problem" with web modules (ES) is that they are incompatible with older package runtimes as they add the export keyword which may not be supported.

So, I support @Aloento view of generating an ES module first and then using rollup to create UMD/CommonJs, because generating them from ES modules is easy whereas generating ES modules from UMD is not feasible.

But in the end this is way more work than any of us should invest for this problem. The great thing is I found this PR: https://github.com/umdjs/umd/issues/124 Which suggests the second edit:

+var getGlobal = function () {
+    if (typeof self !== "undefined") {
+        return self;
+    }
+    if (typeof window !== "undefined") {
+        return window;
+    }
+    if (typeof global !== "undefined") {
+        return global;
+    }
+    throw new Error("unable to locate global object");
+};
+var global = getGlobal();
...
-})(this, () =>
+})(typeof self !== "undefined" ? self : this, () =>

Making it work in angular, and at least loading as web module:

<script type="module">
    import * as dotnet from "./Project/bin/dotnet.js";

    window.onload = async function () {
        await window.dotnet.boot();
        const guestName = window.dotnet.HelloWorld.GetName();
        console.log(`Welcome, ${guestName}! Enjoy your global space.`);
    };
</script>

But running into an error:

dotnet.js:10763 Uncaught (in promise) ReferenceError: bodyJs is not defined
    at Object.bind_method (dotnet.js:10763:49)
    at s (dotnet.js:8876:64)
    at Object.bindings_lazy_init (dotnet.js:8887:49)
    at Object.bind_static_method (dotnet.js:10849:42)
    at p (dotnet.js:13734:38)
    at boot (dotnet.js:13980:34)
    at async Object.exports.boot (dotnet.js:14138:9)
    at async window.onload (global-module.html:9:9)
elringus commented 2 years ago

Making it work in angular

Have you tried [JSFunction] stuff? I guess it won't work there as well. Mono-wasm requires a globally-accessible variable to invoke funcs in JS and looks like while this change allows routing the global into the module it's not persisting the assigned objects back.

DanielHabenicht commented 2 years ago

Making it work in angular

Have you tried [JSFunction] stuff? I guess it won't work there as well. Mono-wasm requires a globally-accessible variable to invoke funcs in JS and looks like while this change allows routing the global into the module it's not persisting the assigned objects back.

Yes, I loaded your example HelloWorld Sample.

elringus commented 2 years ago

Then I'm probably doing something wrong. I've tried using the getGlobal and self -> this stuff, but it still didn't work with the browser-es example. It's booting but assigned functions are not resolved by the runtime.

DanielHabenicht commented 2 years ago

maybe it did not come clear from my comment:

and at least loading as web module:

which means I am running into the same errors with your example, like bodyJs is not defined (on execution of boot(). But at least the loading of the module itself is not erroring. I think this maybe has to with web modules effectively enforcing use strict.

elringus commented 2 years ago

Ok, so I've experimented a bit with migrating the library to es module; rewritten all the JS tests to es-style imports and switched webpack to module output: https://github.com/Elringus/DotNetJS/tree/feat/es-module/JavaScript/dotnet-runtime

I'm currently stuck on how to better wrap the webpack-exported library with the C# packer. Previously I've just copy-pasted the UMD template, but now we have to somehow wrap/override the modules exported by webpack. I'm not familiar enough with all that stuff, so any suggestions are most welcome.

Aloento commented 2 years ago

webpack programmatic api is very useful https://webpack.js.org/api/node/

And Microsoft.AspNetCore.NodeServices can help you to call NodeJS in C# I don't see anything wrong with calling Webpack via MSBuild at compile-time, but it's depends on you.

elringus commented 2 years ago

This will require installing/running webpack when publishing C# project. I'd like to stick with manual patching if possible to simplify the process.

elringus commented 2 years ago

It seems manual patching is not feasible after all, given the mess wepback produce with the module target.

Will investigate AspNetCore.NodeServices.

elringus commented 2 years ago

Ah, looks like it's deprecated. https://github.com/dotnet/AspNetCore/issues/12890

Aloento commented 2 years ago

Let me work on this for your esm