Skip to content

Move to ESM#338

Merged
ai merged 7 commits intoai:mainfrom
lev875:esm
Oct 23, 2023
Merged

Move to ESM#338
ai merged 7 commits intoai:mainfrom
lev875:esm

Conversation

@lev875
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
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.

2 participants