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

Removed priority queue in favor of simple array extension #1157

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

RyanGuild
Copy link

@RyanGuild RyanGuild commented Oct 22, 2024

Description

I began taking appart the app and trying to get a jsconfig.json that would allow me to jump to source going. While working on trying to setup a path to partial esm adoption I was able to process most dependencies by mounting them to the window global but this lib is written in commonjs. I decided to replace it with this couple of lines of ES6.

Motivation

Note there is a <script type="module"> added and the non module scripts reference the module through the globalThis object. I have used this pattern when modernizing large projects to allow for gradual adoption of ESM. ESM leads to clearer module boundaries -> clear module boundaries lead to typedefs -> typedefs lead to typescript. My hope is that I can remove most of these /libs through newer ES features. By my calculations this repo is already using ES2021 features in master so I don't think this is too big a stretch.

Type of change

  • Bug fix
  • New feature
  • Refactoring / style
  • Documentation update / chore
  • Other (please describe)

Versioning

I am not sure how to do the file hash

  • Version is updated
  • Changed files hash is updated

Copy link

netlify bot commented Oct 22, 2024

Deploy Preview for afmg ready!

Name Link
🔨 Latest commit 8a80faf
🔍 Latest deploy log https://app.netlify.com/sites/afmg/deploys/6717ffe9f43a3000081fd1b5
😎 Deploy Preview https://deploy-preview-1157--afmg.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@RyanGuild
Copy link
Author

Hey how do I trigger another netlify build? Also should I be making draft PR's like this?

@Azgaar
Copy link
Owner

Azgaar commented Oct 22, 2024

Hello and thanks for the contribution!

We have flatqueue used in other functions, so we should be using it where we need good performance OR native array methods when it doesn't matter. I don't thing this class is really helpful, also it's very ineffective.

I fully support the modernization effort, but it's already in progress (see #842) and utilizing a bot different approach. There are some core issues that should be solved before switching to es6.

Netlify is automatically rebuilding on commit. But it looks there is a bug in your code, the class method is enqueue, but the method called is queue.

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 this pull request may close these issues.

3 participants