ecs-logging-nodejs icon indicating copy to clipboard operation
ecs-logging-nodejs copied to clipboard

support multiple CommonJS, ESM, and TypeScript import styles

Open trentm opened this issue 3 years ago • 2 comments

Support these import styles:

// 1. `const { ecsFormat } = require('@elastic/ecs-pino-format)` in JS and TS.
//    The preferred import style for JS code using CommonJS.
// 2. `import { ecsFormat } from '@elastic/ecs-pino-format'` in JS and TS.
//    ES module (ESM) import style. This is the preferred style for TypeScript
//    code and for JS developers using ESM.
// 3. `const ecsFormat = require('@elastic/ecs-pino-format')` in JS.
//    The old, deprecated import method. Still supported for backward compat.
// 4. `import ecsFormat from '@elastic/ecs-pino-format'` in JS and TS.
//    This works, but is deprecated. Prefer #2 style.
// 5. `import * as EcsPinoFormat from '@elastic/ecs-pino-format'` is TS.
//    One must then use `EcsPinoFormat.ecsFormat()`

This excludes support for the TypeScript-only "import = require" style:

import ecsFormat = require('@elastic/ecs-pino-format') 

This change adds tests for each import style usage, which should help avoid the (recent, smallish) TypeScript PR rollercoaster.

This approach is based off what I see fastify doing: fastify docs on this and code.

Note that for TypeScript users I think this may be a breaking change. FWIW that TypeScript was only added in the last release (v1.3.0) and was the original intent was more for adding VSCode completion support than about TypeScript code integration.

current status

Just ecs-pino-format so far. If this seems good, I'll do it for winston and morgan as well.

  • [x] ecs-pino-format
  • [x] some sanity check and review
  • [ ] ecs-winston-format
  • [ ] ecs-morgan-format
  • [ ] update preferred import style in docs and examples and tests
  • [ ] changelog entry

trentm avatar Aug 30 '21 22:08 trentm

:green_heart: Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2021-09-29T13:09:14.705+0000

  • Duration: 8 min 26 sec

  • Commit: 1ce41c9a236c257b36b93fa016e1746c55e0c870

:robot: GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

apmmachine avatar Aug 30 '21 22:08 apmmachine

Hi, some news about this pull request ?

BkSouX avatar Feb 19 '22 15:02 BkSouX

FYI I've re-implemented this PR in https://github.com/elastic/ecs-logging-nodejs/pull/165. That should get in early next week.

trentm avatar Oct 27 '23 23:10 trentm