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

refactor this code to use modern code (es6+) and typescript? #1883

Closed
sibelius opened this issue Mar 26, 2020 · 3 comments
Closed

refactor this code to use modern code (es6+) and typescript? #1883

sibelius opened this issue Mar 26, 2020 · 3 comments

Comments

@sibelius
Copy link

are you willing to accept pull requests to migrate incrementally to typescript or more modern code like es6+?

I find kind of "weird" the bits folder (is there all the code is located right now?)

can we also move it inside packages\xlsx?

@SheetJSDev
Copy link
Contributor

When the question was posed years ago, the tooling wasn't great. Older versions of VSCode and github just choke on the xlsx.flow.js script among others. You can see it in action, https://github.com/SheetJS/sheetjs/blob/master/xlsx.flow.js takes seconds to load and the blamelog literally gives you a 504

blamelog 504

The tooling has improved a lot since then, and I think we could start "modernizing" the codebase. The key requirements are preserving IE compatibility in a standalone script and reproducible builds (easy to do when you're just concatenating scripts)

For historical reasons, the project is split up into multiple repos (The original discussion was in the xls repo and transferred to #1834). Now that the two parts are merged in one project, we should move all of the parts into the repo, and I agree that this should live in the packages directory.

So to start, we could probably make a TS version of bits/04_base64.js

@arantespp
Copy link
Contributor

I'd like to help with this refactor. What would be the first step? Starting with the TS version of bits/04_base64.js, the idea is to create a bits/04_base64.ts?

@SheetJSDev
Copy link
Contributor

An ESM build has been added to the repo. It's not 100% yet due to limitations of ES6 import.

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