Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JS: enable circular imports by exporting object (not function) #264

Merged
merged 1 commit into from
Feb 21, 2024

Conversation

generalmimon
Copy link
Member

Resolves kaitai-io/kaitai_struct#1074

This change breaks backward compatibility with 0.10 and older, but allows for circular imports (in all JavaScript module systems) and out-of-order module loading in a "browser globals" context (the latter is particularly relevant in the Web IDE, as explained at kaitai-io/kaitai_struct#1074). In short, it does this by switching from the UMD envelope returnExports.js to modified commonjsStrict.js.

The BC break is that until now the generated modules exported the constructor function directly, whereas now they export the object containing the constructor function under the only object key that matches the format module name. The same behavior is expected from imported opaque types and custom processors as well.

@generalmimon generalmimon force-pushed the js-support-circular-and-out-of-order-imports branch from 9b65fd4 to 02ca87b Compare February 15, 2024 14:23
@generalmimon
Copy link
Member Author

Here's how this compiler change affects the JavaScript code generated from imports0.ksy:

--- 1/compiled-old/javascript/Imports0.js
+++ 2/compiled/javascript/Imports0.js
@@ -3,11 +3,11 @@
 (function (root, factory) {
   if (typeof define === 'function' && define.amd) {
-    define(['kaitai-struct/KaitaiStream', './HelloWorld'], factory);
-  } else if (typeof module === 'object' && module.exports) {
-    module.exports = factory(require('kaitai-struct/KaitaiStream'), require('./HelloWorld'));
+    define(['exports', 'kaitai-struct/KaitaiStream', './HelloWorld'], factory);
+  } else if (typeof exports === 'object' && exports !== null && typeof exports.nodeType !== 'number') {
+    factory(exports, require('kaitai-struct/KaitaiStream'), require('./HelloWorld'));
   } else {
-    root.Imports0 = factory(root.KaitaiStream, root.HelloWorld);
+    factory(root.Imports0 || (root.Imports0 = {}), root.KaitaiStream, root.HelloWorld || (root.HelloWorld = {}));
   }
-}(typeof self !== 'undefined' ? self : this, function (KaitaiStream, HelloWorld) {
+})(typeof self !== 'undefined' ? self : this, function (Imports0_, KaitaiStream, HelloWorld_) {
 var Imports0 = (function() {
   function Imports0(_io, _parent, _root) {
@@ -20,5 +20,5 @@ var Imports0 = (function() {
   Imports0.prototype._read = function() {
     this.two = this._io.readU1();
-    this.hw = new HelloWorld(this._io, this, null);
+    this.hw = new HelloWorld_.HelloWorld(this._io, this, null);
   }
   Object.defineProperty(Imports0.prototype, 'hwOne', {
@@ -33,4 +33,4 @@ var Imports0 = (function() {
   return Imports0;
 })();
-return Imports0;
-}));
+Imports0_.Imports0 = Imports0;
+});

generalmimon added a commit to kaitai-io/kaitai_struct_webide that referenced this pull request Feb 15, 2024
Fixes #59

Fixes #159 -
this is a particularly notable instance of the bug, since `TypeError:
DosDatetime is not a constructor` is an error that initially everyone
gets when they open the Web IDE in a fresh browser (this is because
`zip.ksy` is selected by default, and it is affected by the bug since it
contains an import).

See kaitai-io/kaitai_struct#1074 and
kaitai-io/kaitai_struct_compiler#264 - the
actual fix was done in the KSC-generated JS code, which was changed to
allow interdependent format modules to be loaded (and added to the
global context in the case of the Web IDE) in any order. However, this
was a backwards-incompatible change, so this set of Web IDE changes just
updates our code to work with the new compiler.
generalmimon added a commit to kaitai-io/kaitai_struct_tests that referenced this pull request Feb 16, 2024
Resolves kaitai-io/kaitai_struct#1074

This change breaks backward compatibility with 0.10 and older, but
allows for circular imports (in all JavaScript module systems) and
out-of-order module loading in a "browser globals" context (the latter
is particularly relevant in the Web IDE, as explained at
kaitai-io/kaitai_struct#1074). In short, it
does this by switching from the UMD envelope
[returnExports.js](https://github.com/umdjs/umd/blob/36fd113/templates/returnExports.js#L17-L37)
to modified
[commonjsStrict.js](https://github.com/umdjs/umd/blob/36fd113/templates/commonjsStrict.js#L19-L36).

The BC break is that until now the generated modules exported the
constructor function directly, whereas now they export the object
containing the constructor function under the only object key that
matches the format module name. The same behavior is expected from
imported opaque types and custom processors as well.
@generalmimon generalmimon force-pushed the js-support-circular-and-out-of-order-imports branch from 02ca87b to 1c1a348 Compare February 21, 2024 13:23
@generalmimon generalmimon merged commit 29d37b8 into master Feb 21, 2024
3 of 6 checks passed
@generalmimon generalmimon deleted the js-support-circular-and-out-of-order-imports branch February 21, 2024 13:31
generalmimon added a commit to kaitai-io/kaitai_struct_webide that referenced this pull request Feb 21, 2024
Fixes #59

Fixes #159 -
this is a particularly notable instance of the bug, since `TypeError:
DosDatetime is not a constructor` is an error that initially everyone
gets when they open the Web IDE in a fresh browser (this is because
`zip.ksy` is selected by default, and it is affected by the bug since it
contains an import).

See kaitai-io/kaitai_struct#1074 and
kaitai-io/kaitai_struct_compiler#264 - the
actual fix was done in the KSC-generated JS code, which was changed to
allow interdependent format modules to be loaded (and added to the
global context in the case of the Web IDE) in any order. However, this
was a backwards-incompatible change, so this set of Web IDE changes just
updates our code to work with the new compiler.
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.

JavaScript: change generated API to support circular/out-of-order imports
1 participant