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

feat: update dependencies / general cleanup #1356

Merged
merged 15 commits into from Apr 17, 2020
Merged

feat: update dependencies / general cleanup #1356

merged 15 commits into from Apr 17, 2020

Conversation

@dcodeIO
Copy link
Member

@dcodeIO dcodeIO commented Feb 5, 2020

While looking into the jsdoc issues, especially #1338, I figured that we should instead update all the dependencies. So this PR

  • Updates all the dependencies to their most recent versions (this was surprisingly painless so far)
  • By that, fixes any audit issues
  • Fixes any linting issues
  • Fixes a problem with the main package importing long, which was breaking generated typings
  • Fixes an issue with BufferWriter.alloc not being correctly recognized as a function (aynmore?) during type generation
  • Updates distribution files
  • Reorganizes scripts and runs more of them during CI
  • Adds a workaround for broken top-level enums in jsdoc

On a glimpse everything seems to be working just fine after doing so, but the sheer amount of problems this can theoretically trigger still makes me nervous. So, if anyone here has the nerves to try it and validate that the jsdoc issue is gone, that the dist files still work, or that the CLI doesn't do anything unexpected now, please do :)

@dcodeIO dcodeIO mentioned this pull request Feb 5, 2020
@dcodeIO
Copy link
Member Author

@dcodeIO dcodeIO commented Feb 6, 2020

Regarding the JSDoc problem, from what I can tell now it seems that something broke in JSDoc a while ago related to top-level enums, which is still not fixed entirely, but also doesn't seem as bad as it once was (?). Not sure. There is a workaround in place now for top-level enums specifically (@exports + @enum not having a global scope, but none) that I hope will catch exactly this bug, and become a nop with a potential fix even if we forget it exists.

Also, there's a lot more that could be done in addition to the changes proposed here, like replacing deprecated tslint, continuing to make the CLI its own package and introducing a friendlier code style, but well.

Loading

@Flarna
Copy link

@Flarna Flarna commented Feb 26, 2020

I tried your changes in my project and it looks fine. Did not test that much but at least generation of typescript definitions with node.js 12 works now.

Loading

@dcodeIO dcodeIO mentioned this pull request Mar 11, 2020
@alexander-fenster alexander-fenster changed the title Update dependencies / general cleanup feat: update dependencies / general cleanup Mar 11, 2020
@dcodeIO
Copy link
Member Author

@dcodeIO dcodeIO commented Apr 16, 2020

Got this up to date again, but besides checking OK, it appears there's something strange happening with CI now that I've never seen before?

Loading

@alexander-fenster
Copy link
Contributor

@alexander-fenster alexander-fenster commented Apr 17, 2020

@dcodeIO This is indeed weird. I was able to fix the CI thing by disabling all branch protection for master and then re-enabling it back for the lowercased tasks, which immediately brought the checks for this PR back to normal. I'm guessing, some wires got crossed incorrectly in GitHub Actions?

Anyway, right now the checks look good (all required, and all green), we can move forward!

Loading

@dcodeIO dcodeIO merged commit 42f49b4 into master Apr 17, 2020
9 checks passed
Loading
taylorcode added a commit to taylorcode/protobuf.js that referenced this issue Oct 16, 2020
Co-authored-by: Alexander Fenster <fenster@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants