loader.js runtime `define` and `importSync` interop issue

[chancancode]

It seems like when you have a bare (unimported) define statement, importSync does not work correctly inside the AMD module definition:

import { importSync } from "@embroider/macros";

// eslint-disable-next-line no-undef
define("foo", function() {
  return importSync("foo");
});

This doesn't work. It gets compiled into:

var _embroider_macros_src_addon_es_compat__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(/*! ../../../@embroider/macros/src/addon/es-compat */ "../../@embroider/macros/src/addon/es-compat.js");

// eslint-disable-next-line no-undef
define('foo', function () {
  return esc(require('lodash'));
});

Interestingly, it makes an attempt to require the module containing esc but fails to link them up, also notably, it left the require bare, as opposed to __webpack_require__(...) which would have caused problems even without the esc issue.

On the other hand, this works as expected:

import { importSync } from "@embroider/macros";

globalThis.define("foo", function() {
  return importSync("foo");
});

Which emits:

var _embroider_macros_src_addon_es_compat__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(/*! ../../../@embroider/macros/src/addon/es-compat */ "../../@embroider/macros/src/addon/es-compat.js");

globalThis.define("foo", function () {
  return (0,_embroider_macros_src_addon_es_compat__WEBPACK_IMPORTED_MODULE_0__["default"])(__webpack_require__(/*! lodash * / "../../lodash/lodash.js"));
});

So, it seems like the problem is the bare define call causes some confusion in the build pipeline and left certain things unprocessed.

Not sure what needs to be fixed or turned into a more explicit error, but the current failure mode seemed rather accidental.

This goes hand in hand with #1545 and I think it would be a good idea to provide and official import path for define here.

[chancancode]

For completeness, this is a complete reproduction in an app with a real NPM package, with a bunch of different permutations you can try, and their output/behavior in the comments: https://github.com/chancancode/embroider-define-repro/commit/1cbece3f671b2838231c7278a80aa2b093cfd7cd

[chancancode]

More background: https://discord.com/channels/480462759797063690/1130938499504283860

[ef4]

The problem is that webpack will see AMD and try to handle it.

We already hide bare require from webpack so that we can ensure it keeps its runtime meaning:

https://github.com/embroider-build/embroider/blob/0e00f2bf154bb44448fc7ece0ca3c79a0a08274c/packages/macros/src/babel/macros-babel-plugin.ts#L194-L197

It would be possible to hide require.has and define as well. That is probably what's needed here.

If we do that, it will be equivalent to hand-writing globalThis.define though. If you really need scoped define or require, that would be harder.