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

Migrate to js-yaml also in v2, remove yaml.js from dependencies #168

Open
generalmimon opened this issue Feb 10, 2024 · 0 comments
Open

Comments

@generalmimon
Copy link
Member

generalmimon commented Feb 10, 2024

See #166 (comment) (cc @GreyCat)

#165 discusses switching to js-yaml from yaml.js that we had been using. This was addressed in #166. However, there are still two usages of yaml.js left in the codebase:

  1. "yamljs": "${npmDir}/yamljs/yaml",

    The actual usages to replace (the above line just sets up the yamljs package so that it can be imported) are in this file:

    import { YAML } from "yamljs";

  2. "yamljs": `${npmDir}/yamljs/yaml`,

Both of these places are not used from the v1 version that is currently active. They are associated with the v2 version (see #35). However, I can't get v2 working right now (it seems to be broken), so I did not replace the yaml.js usages there, because I wouldn't have had a way to test it.

On top of that, the src/worker/ImportLoader.ts file seems to be unused even from any v2 code present in the repository, so I guess it could be removed altogether? It appears to be some weird hackish way of handling imports, so we probably shouldn't be using it anyway.

After dealing with these usages, we'll be able to remove yaml.js from vendor.yaml:

# FIXME: remove when no longer referenced by src/KaitaiSandbox.ts and src/worker/ImportLoader.ts
# (see https://github.com/kaitai-io/kaitai_struct_webide/issues/168)
yaml.js:
source: https://github.com/jeremyfa/yaml.js
licenseName: MIT
licenseUrl: https://raw.githubusercontent.com/jeremyfa/yaml.js/develop/LICENSE
npmDir: yamljs
files: [LICENSE, dist/yaml.js]

This will cause yaml.js to no longer be available under the path lib/_npm/yamljs. Then the yamljs npm package won't actually be used anymore, so it should be removed from package.json as well:

"yamljs": "^0.3.0"

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

No branches or pull requests

1 participant