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

Release new version for updated dependencies? #70

Open
evanpurkhiser opened this issue May 23, 2021 · 2 comments
Open

Release new version for updated dependencies? #70

evanpurkhiser opened this issue May 23, 2021 · 2 comments

Comments

@evanpurkhiser
Copy link

loader-utils are required afaict, but are not included in the current npm versions dependencies list.

evanpurkhiser added a commit to evanpurkhiser/prolink-connect that referenced this issue May 23, 2021
This dependency is missing in the kaitai-struct-loaders dependencies. kaitai-io/kaitai_struct_loader#70
@generalmimon
Copy link
Member

You're right, the v0.9.0 tag doesn't declare loader-utils in package.json dependencies:

"dependencies": {
"js-yaml": "^3.13.1",
"kaitai-struct": "^0.9.0",
"kaitai-struct-compiler": "^0.9.0"
}

but uses it:

var loaderUtils = require("loader-utils");

Which is obviously an error. However, if I'm supposed to release a new version, I should really fix a few things - first, there is a growing queue of Dependabot pull requests that I should merge so that any known vulnerabilities get fixed in the dependencies. The only reason why I have been leaving them unmerged is that I got an impression from this issue kaitai-io/kaitai_struct#729 that it's desirable for our users that we maintain compatibility with the lowest possible version of Node.js that is installed on some LTS Linux distribution (I don't like and understand it much, I'd rather support only Node.js versions that aren't EOL, which would mean Node.js 12 and 14 at the moment - that would make life so much easier. But I'm no expert on Node.js, I don't use it in any serious application, so I don't dare to make such bold decisions like that on my own.). That's why I was shooting for Node.js {4,8,10,12,14} at first, but then found that Node.js 4 doesn't work with yarn, so I raised it to Node.js 6. And I'm sure that some of the dependency updates suggested by Dependabot would break some of the versions (they would probably break compatibility at least with Node.js 4 and 8), so I was not merging them right away and I've been hoping to find some time to try locally if these dependencies break the compatibility or not.

Also, I don't know if I need to use a united old dependency version compatible with Node.js 6 for all Node.js versions (even for Node.js 14, which could run something much newer) or whether there is some way to declare different dependency versions for different Node.js versions (but I don't know if e.g. Webpack 5 won't use a different syntax that will make it incompatible with current codebase). Do you see that "lowest possible version" madness?

The second thing that I wanted to fix are relative imports. I thought that the PR #5 added them, but I found that it has never actually worked as soon as I introduced the unit testing in the commit 684faf4. I tried to fix it, but I essentially got stuck because I'm not that proficient in writing Webpack plugins. IIRC, the problem is that the Kaitai Struct compiler is handling the imports itself, which Webpack does not expect - when KSC sees a meta/imports declaration, it requests the appropriate .ksy spec from the filesystem, processes it and at the end returns a map with generated JS module filename as a key (e.g. HelloWorld.js) and the JS code as a value. However, Webpack does not work like that - when you write import DoomWAD from './game/doom_wad.ksy', Webpack reads contents of that file and gives it as a source code to the kaitai_struct_loader plugin. In return, it expects a single JavaScript module (generated purely from that source code that it provided), then processes it, searches for any import ... or require(...) statements (and the KS-generated modules with imports always contain require('./HelloWorld') for meta/imports: [hello_world]) and then it gets mad because there is no HelloWorld.js file there. I haven't yet figured out how to adapt the kaitai_struct_loader for this or how to change Webpack behavior.

@Ithamar
Copy link
Contributor

Ithamar commented Jun 26, 2021

Regarding the issue with the automatic CamelCasing of requires by the KSY compiler, it looks like if you stick to CamelCasing the actual KSY files, the requires will match the filenames and webpack will figure out things (you ofcourse also need to CamelCase the import statement inside the ksy, though the type you import will be based on the meta-id field from the KSY file, so it will have the "proper" ksy lowercase naming)

This is really a gross hack, and IMHO the compiler should simply not CamelCase the requires, or at least offer an option to not do that.

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

No branches or pull requests

3 participants