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

Avoid stream polyfill for GFF3/GTF parsing #4547

Merged
merged 1 commit into from
Sep 4, 2024
Merged

Avoid stream polyfill for GFF3/GTF parsing #4547

merged 1 commit into from
Sep 4, 2024

Conversation

cmdcolin
Copy link
Collaborator

@cmdcolin cmdcolin commented Sep 4, 2024

Fixes GMOD/gff-js#62

I extracted the non-streaming parts of the gff and gtf parsers into their own NPM packages that can be used in isolation, and called the packages gff-nostream and gtf-nostream

This reduces the need for node.js polyfills and the stream-browserify workaround

Potentially, even the streaming parts of @gmod/gff could use gff-nostream as a base package to avoid the code duplication, but right now, this would just remove @gmod/gff and @gmod/gtf entirely from the build in favor of the non-streaming new packages

note: the naming gff-nostream is a little awkward but was hard to come up with something clearly named to distinguish it from @gmod/gff

saves about 50kb ungzipped/15kb gzipped js

@cmdcolin cmdcolin force-pushed the nostream branch 3 times, most recently from dd5b27c to 1bf74cc Compare September 4, 2024 16:30
@cmdcolin
Copy link
Collaborator Author

cmdcolin commented Sep 4, 2024

The parseStringSync now only return features with no "options" object to be otherwise. reason being

  • jbrowse doesn't use comments/sequences/directives
  • the other types complicated the typescript (long parseStringSync types in @gmod/gff hints to this)
  • not just the typescript but runtime interpretation is challenging with other types because it is just a linear array of "GFF3Item = GFF3Feature | GFF3Directive | GFF3Comment | GFF3Sequence" where there is no stringly typed tag field to really distinguish them easily
  • included legacy options like disableDerivesFromReferences

this could be considered to be done somewhat different to add more options, but i think this makes the default behavior best for jbrowse usage

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

Successfully merging this pull request may close these issues.

Proposal: Modularize out a non-stream version of the package
1 participant