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

Move to ESM #338

Merged
merged 7 commits into from
Oct 23, 2023
Merged

Move to ESM #338

merged 7 commits into from
Oct 23, 2023

Conversation

lev875
Copy link
Contributor

@lev875 lev875 commented Oct 22, 2023

#227

  • Added type: module to all package.json files
  • Swapped jest for vitest
    • Updated snapshots
    • Updated tests
    • Split fixtures in esbuild and webpack plugins into cjs and esm directories
      • Both esbuild and webpack are determining the module type based on nearest package.json file. When I switched these plugins to type: module in their respective package.json files it broke some tests, because the bundlers started treating the files as esm and not cjs like before. That's why I've split the fixtures directories and added package.json files to force the correct module type during bundling tests.
    • Added tests for esbuild and webpack using esm configs
  • Updated code to esm
    • Rewrote all imports to esm style
    • Rewrote loadPlugins to use promise based dynamic import() instead of require()
    • Rewrote package.json import in create-help.js using createRequire to avoid using experimental import ... assert syntax
  • Updated the ci workflow

Notes:

  • Some tests are generating temporary files inside 'fixtures' directory. If you do not exclude 'fixtures' directory in vitest watch path, it will result in tests infinitely triggering reruns in watch mode.
  • I took a liberty of rewriting a couple of tests from if (stmt) { it(...) } to it.skipIf(!stmt)( ... ) so it looks a bit cleaner and the runner reports these tests as skipped

import open from 'open'
import {join} from 'path'
import rm from 'size-limit/rm'
import { afterEach, expect, it, vi } from "vitest"
Copy link
Owner

Choose a reason for hiding this comment

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

Can you run Prettier on changed files?

I have it in VS Code. But you can install to the project, run, and uninstall.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, damn, I did eslint --fix but forgot about prettier. I've installed it globally and ran pnpm exec prettier **/*.js --write, should do the trick.

})

describe('throws error on unknown entry', () => {
it('should work with commonjs config', async () => {
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
it('should work with commonjs config', async () => {
it('works with commonjs config', async () => {

and all other tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed by c17c150

@ai ai mentioned this pull request Oct 23, 2023
19 tasks
@ai ai merged commit a015e3e into ai:main Oct 23, 2023
2 checks passed
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.

None yet

2 participants