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

Re: Re: Move to ESM #106

Merged
merged 32 commits into from
Dec 28, 2021
Merged

Re: Re: Move to ESM #106

merged 32 commits into from
Dec 28, 2021

Conversation

mizdra
Copy link
Owner

@mizdra mizdra commented Dec 27, 2021

ref: #84, #85

BREAKING CHANGES

  • Drop node < 14.3.1

Tasks

Phase 1: Read guides

The conversion to ESM can cause a lot of BREAKING CHANGES. Also, support for various toolchains is partial. I will read this guide and check if I can really convert eslint-interactive to ESM and not have any problems.

Phase 2: Check support status of toolchains

At this time, many toolchains have partial support for ESM. While many applications can be converted to ESM, some may not be able to be converted due to lack of support in the toolchain. Therefore, it is necessary to check the support status of various toolchains.

Phase 3: Setup

Add some basic configs.

  1. Add "type": "module" in package.json: 1d0756d
  2. Update typescript@nightly: cae48aa
  3. Use nightly version tsserver in VSCode: 44a1054
  4. Change "module": "node12" and "moduleResolution": "node12" of tsconfig.json: 03e2117

Phase 4: Fix type errors

Fix type errors by checking the results of yarn run lint:tsc.

  1. Change import ... from './path/to/module'; to import ... from './path/to/module.js';: 888b431
    • It is because ESM does not allow the omission of the extension.
  2. Change import ... from './path/to/dir'; to import ... from './path/to/dir/index.js';: bfd8558
    • It is because ESM does not allow the omission of index.js.
  3. Update dependencies to supported versions of ESM: 6fb215f, 25a66f0
    • Depending on the situation, CJS packages may not be importable with the import statement (import ... from '...';).
    • If it is an ESM package, it can be easily imported with an import statement.
    • So, update dependencies to supported versions of ESM.
  4. Import non-ESM (CJS) packages with the import ... = require('...') syntax: ab398aa, be4d269
    • Import the rest of the packages using require

Phase 5: Fix lint errors

Fix lint errors by checking the results of yarn run lint:eslint.

  1. Rename .eslintrc.js to .eslintrc.cjs: b8cfc2e
  2. Disable import/no-unresolved rule: 9e91c7c
  3. Include .cjs, .mjs, .cts and .mts as lint targets: 36e4c54, d97a2b4

Phase 6: Fix runtime errors

Fix runtime errors by checking the results of yarn run dev.

  1. Add an extension (.js) to the path in the bin field of package.json: a932cb1
  2. Import the CJS package with ESM-interop: 69595d7
    • Node.js output a hint at runtime, so follow it to fix the problem.
    • $ yarn run dev
      file:///Users/mizdra/src/github.com/mizdra/eslint-interactive/dist/cli/prompt.js:2
      import { prompt } from 'enquirer';
               ^^^^^^
      SyntaxError: Named export 'prompt' not found. The requested module 'enquirer' is a CommonJS module, which may not support all module.exports as named exports.
      CommonJS modules can always be imported via the default export, for example using:
      
      import pkg from 'enquirer';
      const { prompt } = pkg;
      
      at ModuleJob._instantiate (node:internal/modules/esm/module_job:127:21)
  3. Convert __dirname to dirname(fileURLToPath(import.meta.url)): 6db3b65
    • __dirname is not supported in ESM
  4. Fix mismatch between type definitions and runtime types: c5dc7b0
    • I don't know the cause, but sometimes the type definition and the runtime do not match.
    • I used a cast to force a fix.
  5. Remove module.exports: 0a980ad
    • eslint-interactive used module.exports in some packages to support both CJS/ESM.
    • This will cause a runtime error since module.exports is not supported in ESM.
    • Since the whole eslint-interactive will be Pure ESM, this hack will no longer be necessary.
    • So, I will remove it.
  6. Make the entire fixtures/ CJS: bca4371
    • For testing purposes, fixtures/ must be treated as CJS.
    • So I put fixtures/package.json with {"type": "commonjs"}.
    • This treats fixtures/ as CJS.

Phase 7: Fix tests

Fix tests by checking the results of yarn run test.

  1. Convert jest.config.js to jest.config.mjs: cdc1672
  2. Update jest toolchain: 1afa2db
  3. Setup jest config to support ESM: c550aec
  4. Use custom resolver: f5f57f0, 9a33888
  5. Insert import { jest } from '@jest/globals';: deea149, 7683a9d
  6. Replace jest.requireActual with another solutions: 009081e, 45b5015
    • jest.requireActual is not yet supported in ESM.
  7. Include .cjs, .mjs, .cts and .mts as coverage targets: 2b4bd5e

Phase 8: Refactor codes

Refactoring bad code.

  1. Use ESM version of comlink: 19016e1
    • comlink provides an ESM module (ex. comlink/dist/esm/node-adapter.mjs)
    • Of course, comlink also provide type definitions (ex. comlink/dist/esm/node-adapter.d.ts)
    • However, the corresponding type definition for xxx.mjs is xxx.d.mts in "module": "node12" mode
    • Therefore, importing the ESM module causes compilation errors
    • So, use paths option of tsconfig.json to force xxx.mjs to be mapped to xxx.d.ts.

Phase 9: Confirm operation

  1. npm pack
  2. npm install -D xxx.zip
  3. Confirm operation of the package
    • Confirm on each OS and Node.js version supported by eslint-interactive

TODO

  • Confirm operation
  • Merge this PR
  • Release new version
  • Report problems of toolchains that I found in this work
    • It is a good idea to send feedback for the future of the ecosystem 👍
    • Compatibility issues with type definitions of comlink

@mizdra mizdra added Type: Breaking Change Includes breaking changes Type: Maintenance Repository Maintenance labels Dec 27, 2021
@mizdra
Copy link
Owner Author

mizdra commented Dec 27, 2021

It works! It's perfect...

@mizdra mizdra changed the title Re-re-challenge: Use module: node12 in tsconfig.json Re: Re: Use module: node12 in tsconfig.json Dec 28, 2021
@mizdra mizdra changed the title Re: Re: Use module: node12 in tsconfig.json Re: Re: Move to ESM Dec 28, 2021
@mizdra
Copy link
Owner Author

mizdra commented Dec 28, 2021

Everything is ready to go. We're moving forward!

@mizdra mizdra merged commit 804a7b9 into main Dec 28, 2021
@mizdra mizdra deleted the use-module-node12-3 branch December 28, 2021 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Breaking Change Includes breaking changes Type: Maintenance Repository Maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant