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

feat: support multiple import styles from JS and TS; add morgan types #165

Merged
merged 1 commit into from
Oct 30, 2023

Conversation

trentm
Copy link
Member

@trentm trentm commented Oct 27, 2023

This updates the three @elastic/ecs-*-format packages to support the following
import styles from JS and TS code:

  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 style (2).
  5. import * as EcsPinoFormat from '@elastic/ecs-pino-format'; in TS.
    One must then use EcsPinoFormat.ecsFormat().

Note that this excludes support for this TS-only style:
import escFormat = require('@elastic/ecs-pino-format');

This also adds types for ecs-morgan-format, based on #119 and #90.
I'd had an earlier start on this in #96.

Replaces: #96
Closes: #90
Closes: #119


The functional part of this diff is this in each index.js file:

+// For backwards compatibility with v1.0.0, the top-level export is `ecsFormat`,
+// though using the named export is preferred.
 module.exports = ecsFormat
+module.exports.ecsFormat = ecsFormat
+module.exports.default = ecsFormat

plus the new morgan/index.d.ts file. The rest of the diff is tests, style updates,
and a npm run test:skip-slow convenience for dev. Note that I added exhaustive
tests just for ecs-pino-format. I thought it overkill to add "import-styles.test.js"
for the other packages as well.

This updates the three @elastic/ecs-*-format packages to support the following
import styles from JS and TS code:
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';` in TS.
   One must then use `EcsPinoFormat.ecsFormat()`.

Note that this *excludes* support for this TS-only style:
`import escFormat = require('@elastic/ecs-pino-format');`

This also adds types for ecs-morgan-format, based on #119 and #90.
I'd had an earlier start on this in #96.

Replaces: #96
Closes: #90
Closes: #119
@trentm trentm requested a review from david-luna October 27, 2023 22:52
@trentm trentm self-assigned this Oct 27, 2023
@github-actions github-actions bot added the agent-nodejs Make available for APM Agents project planning. label Oct 27, 2023
Copy link
Member

@david-luna david-luna left a comment

Choose a reason for hiding this comment

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

All good :) just curious about a couple of things in the docs

@trentm trentm merged commit af2f9e9 into main Oct 30, 2023
@trentm trentm deleted the trentm/ts branch October 30, 2023 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-nodejs Make available for APM Agents project planning.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants