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

Convert @zooniverse/async-states to ES6 #3920

Closed
wants to merge 1 commit into from

Conversation

eatyourgreens
Copy link
Contributor

Switch @zooniverse/async-states from require to static import and export. Publish the package as ESM, not CommonJS.

Tests fail in consuming packages, because of #3779.

Package

lib-async-states

How to Review

Build and run both the project app and the content pages app. Both apps should be unchanged.

Checklist

PR Creator - Please cater the checklist to fit the review needed for your code changes.
PR Reviewer - Use the checklist during your review. Each point should be checkmarked or discussed before PR approval.

General

  • Tests are passing locally and on Github
  • Documentation is up to date and changelog has been updated if appropriate
  • You can yarn panic && yarn bootstrap or docker-compose up --build and FEM works as expected
  • FEM works in all major desktop browsers: Firefox, Chrome, Edge, Safari (Use Browserstack account as needed)
  • FEM works in a mobile browser

General UX

Example Staging Project: i-fancy-cats

  • All pages of a FEM project load: Home Page, Classify Page, and About Pages
  • Can submit a classification
  • Can sign-in and sign-out
  • The component is accessible

Refactoring

  • The PR creator has described the reason for refactoring
  • The refactored component(s) continue to work as expected

@eatyourgreens eatyourgreens added the refactor Refactoring existing code label Nov 28, 2022
@eatyourgreens eatyourgreens requested a review from a team November 28, 2022 15:21
@eatyourgreens eatyourgreens added the experiment Something to try out and gather more information on label Nov 28, 2022
@eatyourgreens eatyourgreens force-pushed the es6-async-states branch 2 times, most recently from 3116501 to c9ec2b6 Compare November 28, 2022 21:17
@eatyourgreens
Copy link
Contributor Author

@zooniverse/async-states is a single JS file and tests, so it seems like a good candidate to try running the Mocha tests with export default rather than module.exports =.

@eatyourgreens
Copy link
Contributor Author

eatyourgreens commented Nov 28, 2022

Tests replace import with require, which fails with this error:

Error [ERR_REQUIRE_ESM]: require() of ES Module 
/home/runner/work/front-end-monorepo/front-end-monorepo/packages/lib-async-states/src/async-states.js 
from 
/home/runner/work/front-end-monorepo/front-end-monorepo/packages/app-project/stores/Store.js 
not supported.
Instead change the require of async-states.js in 
/home/runner/work/front-end-monorepo/front-end-monorepo/packages/app-project/stores/Store.js 
to a dynamic import() which is available in all CommonJS modules.

Switch `@zooniverse/async-states` from `require` to static `import` and `export`.
@eatyourgreens eatyourgreens deleted the es6-async-states branch November 22, 2023 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experiment Something to try out and gather more information on refactor Refactoring existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants