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

YAML.parseAllDocuments is no longer accessible #863

Closed
Gabswim opened this issue Jun 26, 2024 · 10 comments · Fixed by #866
Closed

YAML.parseAllDocuments is no longer accessible #863

Gabswim opened this issue Jun 26, 2024 · 10 comments · Fixed by #866

Comments

@Gabswim
Copy link

Gabswim commented Jun 26, 2024

Expected Behavior

https://stackblitz.com/edit/zx7-2-3-yaml?file=src/main.ts&terminal=dev

import { YAML } from 'zx'
YAML.parseAllDocuments() // this function is accessible

Actual Behavior

https://stackblitz.com/edit/zx-8-1-3-yaml?file=src/main.ts&terminal=dev

import { YAML } from 'zx'
YAML.parseAllDocuments() // this function is not accessible

Steps to Reproduce

  1. Update from 7.2.3 to 8.1.3.

Specifications

  • zx version: 8.1.3
  • Platform: macos
  • Runtime: node
@antongolub
Copy link
Collaborator

antongolub commented Jun 26, 2024

This is related to TS case, right?

  1. The issue can be fixed, but it will also add ~50Kb to dts libdefs.
  2. If we'd like to replace the current YAML engine with another one (smaller, faster, etc), no chance that it will also provide support for parseDocument/parseAllDocuments

CC @antonmedv

@antonmedv
Copy link
Collaborator

This is because we bundle yaml?

@antongolub
Copy link
Collaborator

Because we've omitted the API typings except parse and stringify.

@antonmedv
Copy link
Collaborator

But the parseAllDocuments code is still in the bundle?

@Gabswim
Copy link
Author

Gabswim commented Jun 26, 2024

I have a workaround: installing the yaml library directly if the goal is to restrain the API to a smaller footprint.

But the parseAllDocuments code is still in the bundle?

Yes, so right now the typing is not showing the reality of what is being accessible.
Maybe what you can do is something like this

import { parse, stringify } from 'yaml'

export const YAML = {
  parse,
  stringify
}

@antongolub
Copy link
Collaborator

antongolub commented Jun 26, 2024

But the parseAllDocuments code is still in the bundle?

Correct. In js bundle.

@antonmedv
Copy link
Collaborator

But why ts def grows so much?

@antongolub
Copy link
Collaborator

External contracts are derived from internal ones. There are actually many many types.
https://www.npmjs.com/package/yaml?activeTab=code

@tobias4711gg
Copy link

but when changing from YAML.parseDocument to YAML.parse all comments in the YAML file are lost!

@antongolub
Copy link
Collaborator

@tobias4711gg,

We've reverted this change: #879

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 a pull request may close this issue.

4 participants