Skip to content

refactor issue finder to replace vue with plain js and update html#868

Merged
TimidRobot merged 5 commits intocreativecommons:vocab-refreshfrom
Queen-codes:issue-finder
Jan 10, 2025
Merged

refactor issue finder to replace vue with plain js and update html#868
TimidRobot merged 5 commits intocreativecommons:vocab-refreshfrom
Queen-codes:issue-finder

Conversation

@Queen-codes
Copy link
Copy Markdown
Contributor

Fixes

Description

This pull request refactors the Issue Finder by removing Vue.js and simplifying the codebase to use plain js. the issue-finder.html file was updated to align with the refactored JavaScript code

Checklist

  • My pull request has a descriptive title (not a vague title like Update index.md).
  • My pull request targets the default branch of the repository (main or master).
  • My commit messages follow best practices.
  • My code follows the established code style of the repository.
  • I added or updated tests for the changes I made (if applicable).
  • I added or updated documentation (if applicable).
  • I tried running the project locally and verified that there are no
    visible errors.

Developer Certificate of Origin

For the purposes of this DCO, "license" is equivalent to "license or public domain dedication," and "open source license" is equivalent to "open content license or public domain dedication."

Developer Certificate of Origin
Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

@Queen-codes Queen-codes requested review from a team as code owners December 30, 2024 11:09
@Queen-codes Queen-codes requested review from Shafiya-Heena, TimidRobot and akmadian and removed request for a team December 30, 2024 11:09
@Queen-codes
Copy link
Copy Markdown
Contributor Author

Hi @TimidRobot

Progress So Far

  • I've refactored issue-finder.js to plain JavaScript and maintained the core functionality. There are still some minor tweaks needed, specifically with the skills dropdown options, as they’re not displaying as expected. I'll try and continue working on this.

  • I attempted to move the issue-finder.js file into the static folder and included it in both layout.html and issue-finder.html using a script tag, but it didn’t work as intended. If there’s an alternative approach you’d recommend, I’d love to try it.

  • Since hydration.js and components.js are no longer necessary for functionality, is it okay to remove these files entirely from the project?

  • I checked all the Vocabulary specimens but couldn’t find one that aligns well with the design and functionality of the issue finder. If you could point me toward a suitable pattern or specimen, I’d really appreciate it.

@TimidRobot
Copy link
Copy Markdown
Member

@Queen-codes, response inline:

Progress So Far

  • I've refactored issue-finder.js to plain JavaScript and maintained the core functionality. There are still some minor tweaks needed, specifically with the skills dropdown options, as they’re not displaying as expected. I'll try and continue working on this.

Sounds good!

  • I attempted to move the issue-finder.js file into the static folder and included it in both layout.html and issue-finder.html using a script tag, but it didn’t work as intended. If there’s an alternative approach you’d recommend, I’d love to try it.

My expectation is that it should only need to be included from issue-finder.html.

Putting it in assets/static/js/issue-finder.js and using <script src="/static/js/issue-finder.js"></script> seems to load it fine for me. I get the following error:

Uncaught SyntaxError: import declarations may only appear at top level of a module
  • Since hydration.js and components.js are no longer necessary for functionality, is it okay to remove these files entirely from the project?

Yes! I'm looking forward to decreasing the JavaScript complexity/requirements!

  • I checked all the Vocabulary specimens but couldn’t find one that aligns well with the design and functionality of the issue finder. If you could point me toward a suitable pattern or specimen, I’d really appreciate it.

I'm not aware of any, either. I would follow strategy of keeping HTML and removing site-specific CSS.

@TimidRobot TimidRobot self-assigned this Jan 6, 2025
@Queen-codes
Copy link
Copy Markdown
Contributor Author

Hi @TimidRobot and @possumbilities . I hope you're both doing well.

I've encountered some issues after moving the issue-finder.js file from webpack to the assets/static/js folder. Here are the errors I'm encountering:

  • Module not found: Error: Can't resolve './js/issue-finder.js' in '/Users/QueenIslamiat/Desktop/fe/cc/creativecommons.github.io-source/webpack'.

  • When I included the script tag in issue-finder.html, I encountered this error:
    Uncaught SyntaxError: Cannot use import statement outside a module (at issue-finder.js:1:1)(the same TimidRobot mentioned above).
    I tried adding type="module" to the script tag, and it cleared the error message, but it led to a new issue.

  • After adding type="module", I received this error:
    Uncaught TypeError: Failed to resolve module specifier "@octokit/rest". Relative references must start with either "/", "./", or "../".
    I think this is happening because @octokit/rest is a Node.js module and needs to be bundled with a bundler or served with a cdn. I'm not entirely sure.

I'd appreciate any help and guidance on how I can resolve this, or if there's anything I'm missing. thank you.

@TimidRobot
Copy link
Copy Markdown
Member

Hi @TimidRobot and @possumbilities . I hope you're both doing well.

I've encountered some issues after moving the issue-finder.js file from webpack to the assets/static/js folder. Here are the errors I'm encountering:
[...]

I think we've encountered the limits of my node/NPM knowledge. @possumbilities, please advise.

@possumbilities
Copy link
Copy Markdown
Contributor

possumbilities commented Jan 8, 2025

@Queen-codes @TimidRobot I'll look into it further, but one thing I noticed immediately is that this may not be the entire issue, but is definitely one of them:

I think this is happening because @octokit/rest is a Node.js module and needs to be bundled with a bundler or served with a cdn. I'm not entirely sure.

Serving modules directly absent a bundler is not an entirely new thing, but it is more newly "stable" in a lot of environments. That said, octokit itself provides a useful example of the difference of inclusion between utilizing something like npm vs direct script tags. I'd suggest starting there if you are to avoid the use of Webpack here. However, their use of a direct module is pulling it via a CDN, which means it hasn't been localized to the project. Also, once you use "in script tag imports" you are entering the territory of how browser's have implemented ES6 Module imports and errors may occur differently depending on the browser + operating system, since the "transpilation steps" are not happening prior to page load, but within the browser itself.

There's a lot more on ES6 module imports here: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Modules

See also the GitHub Repo of octokit, where it also has similar inclusion instructions.

I'll keep looking to see if there's something I've missed that might be more helpful.


If your JS is going to depend on another JS file, and its just one file then avoiding the use of a package manager/bundler is usually a good idea. However, if your file is going to depend on a file that depends on other files and so on you start to get into territory where historically something like npm or yarn becomes worthwile to keep up with all that. ES6 modules provide a means (should browsers support it) to skip something like Webpack and instead let the browser "transpile" the dependency chain, but that means the files need to live somewhere; often the default assumption is that they'll live on a cdn (with some form of import map there). This may not be ideal if you want to localize all the files in a semi-hermetic way. To do that you'd need the files, and all the downstream dependencies to exist within the project somewhere, so then you're back to needing some kind of package manager in the chain to keep things up to date, track dependency chains, etc. Generally, the cdn route let's you bypass this, BUT at the cost of having to download something from a third-party instance at page load; and that can increase security surface area in ways that might not be desired.

@possumbilities
Copy link
Copy Markdown
Contributor

@Queen-codes Coming back to say that I think the most direct route at the moment is to remove Vue.js, but not try to escape Webpack at the moment. It might be worthwhile exploring later in this project or as a future sub-project; but given the complexity it might require to pull it off in a stable way I think the larger concern should be prioritizing removing Vue.js, and correcting the HTML to be both in line with the spirit and direction of Vocabulary.

So I would say to not move the issue-finder.js at this time, doing so seems like scope creep to me.

@Queen-codes
Copy link
Copy Markdown
Contributor Author

okay, thank you. this was helpful

Copy link
Copy Markdown
Member

@TimidRobot TimidRobot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good work, thank you!

@TimidRobot TimidRobot merged commit 4e02f17 into creativecommons:vocab-refresh Jan 10, 2025
@TimidRobot TimidRobot linked an issue Jan 10, 2025 that may be closed by this pull request
1 task
@Queen-codes Queen-codes deleted the issue-finder branch January 11, 2025 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project
Archived in project

Development

Successfully merging this pull request may close these issues.

[Feature] Remove vue and clean-up javaScript in CC Open Source

3 participants