Skip to content

fix: output esm into "*.mjs" module#47

Merged
kettanaito merged 1 commit intomswjs:mainfrom
mattcosta7:mjs-extensions
Oct 6, 2022
Merged

fix: output esm into "*.mjs" module#47
kettanaito merged 1 commit intomswjs:mainfrom
mattcosta7:mjs-extensions

Conversation

@mattcosta7
Copy link
Copy Markdown
Contributor

Removes legacyOutput option from tsup, in favor the mjs output used without that configuration.

removes unnecessary esm folder and package.json

output of lib with this config (to compare against).

Screen Shot 2022-10-05 at 8 49 38 PM

I haven't published/tested node resolution here - this might be helpful before versioning/merging

@mattcosta7
Copy link
Copy Markdown
Contributor Author

mattcosta7 commented Oct 6, 2022

Tests I did locally

  • start a repo with
  • install a version of headers-polyfill
  • override the package.json based on these changes
  • override the built files based on these changes
  • install jest
  • write a simple test
import { Headers } from 'headers-polyfill'
it('works', () => {
    expect(Headers).toBeDefined()
})
  • run jest, without experimental modules flag (using base transform)
  • provide an empty transform object in jest config
  • run jest, with experimental modules flag.

that all seems to check out, at least using node 16.17.1

without type: module, using

const {Headers} = require('headers-polyfill')
it('works', () => {
    expect(Headers).toBeDefined()
})

seems to work fine

as does a ts file, using tsjest

import { Headers } from 'headers-polyfill'
it('works', () => {
    expect(Headers).toBeDefined()
})

@mattcosta7 mattcosta7 marked this pull request as ready for review October 6, 2022 01:31
Copy link
Copy Markdown
Member

@kettanaito kettanaito left a comment

Choose a reason for hiding this comment

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

This looks great. Thank you @mattcosta7!
I'm glad to be rid of the /esm folder. Let's see the CI status before merging.

@kettanaito
Copy link
Copy Markdown
Member

I love GitHub Actions sometimes

Error: Input required and not supplied: token

@kettanaito kettanaito changed the title use mjs instead of esm folder fix: output esm into "*.mjs" module Oct 6, 2022
@mattcosta7
Copy link
Copy Markdown
Contributor Author

I love GitHub Actions sometimes

Error: Input required and not supplied: token

Yeah, I think this is probably because it's a fork, which doesn't have access to repo secrets (iirc)

@mattcosta7
Copy link
Copy Markdown
Contributor Author

@kettanaito

https://github.com/mswjs/headers-polyfill/blob/main/.github/workflows/ci.yml#L19

This line in the workflow is the error. Secrets aren't exposed on most actions in forks to avoid users maliciously stealing them

Using pull_request_target events should allow secrets from forks (I believe), but comes with the drawback that the version of the action from the base branch runs instead of the head (so changes need to land or be branch off to validate)

@kettanaito kettanaito merged commit a0d416b into mswjs:main Oct 6, 2022
@kettanaito
Copy link
Copy Markdown
Member

Released: v3.1.1 🎉

This has been released in v3.1.1!

Make sure to always update to the latest version (npm i headers-polyfill@latest) to get the newest features and bug fixes.


Predictable release automation by @ossjs/release.

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