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

Import attributes break message exporting #4489

Closed
brawaru opened this issue Aug 7, 2024 · 6 comments
Closed

Import attributes break message exporting #4489

brawaru opened this issue Aug 7, 2024 · 6 comments

Comments

@brawaru
Copy link

brawaru commented Aug 7, 2024

Which package?

@formatjs/[email protected]

Describe the bug

When import attributes are used in the code, @formatjs/cli fails to extract messages despite tsc being able to successfully check the file.

The following errors are printed to console:

[@formatjs/cli] [WARN] Error: index.mts:2:43 - error TS1005: ';' expected.

2 import { doSomething } from './other.mjs' with { type: 'macro' }
                                            ~~~~
index.mts:2:48 - error TS1005: '(' expected.

2 import { doSomething } from './other.mjs' with { type: 'macro' }
                                                 ~
index.mts:4:1 - error TS1005: ')' expected.

4 export const msg = defineMessage({
  ~~~~~~

To Reproduce

Codesandbox URL

https://stackblitz.com/edit/node-by7zrp?file=package.json

Reproducible Steps/Repo

  1. Create file b.mts with contents like:
export function myMacro() {
  return { id: 'woo' }
}
  1. Create file a.mts with contents like:
import { defineMessage } from '@formatjs/intl'
import { myMacro } from './b.mts' with { type: 'macro' }

export const greeting = defineMessage({
  id: 'greeting',
  defaultMessage: 'Hello, {name}!',
})

export const smthnElse = myMacro()
  1. Try to extract messages from a.mts
pnpm formatjs extract a.mts --out-file output.json

Expected behavior

Extraction completes without errors and yields a file output.json with contents:

{
  "greeting": {
    "defaultMessage": "Hello, {name}!"
  }
}

Screenshots

N/A

Desktop (please complete the following information):

N/A

Smartphone (please complete the following information):

N/A

Additional context

N/D

@brawaru brawaru added the bug label Aug 7, 2024
@evert
Copy link

evert commented Aug 8, 2024

Also just ran into this! We actually used the assert keyword instead of with, but starting Node 22 assert is dropped. Honestly kinda surprising to see a non-flagged stable feature dropped in Node.

nodejs/node#52104

I'm downgrading to Node 20 as we speak, as formatjs extract not breaking trumps being on Node 22

@brawaru
Copy link
Author

brawaru commented Aug 8, 2024

I believe the solution is to upgrade TypeScript package used by @formatjs/cli-lib and then add module: ts.ModuleKind.Preserve to compilerOptions. Preserve will allow to be import-agnostic, supporting both require and import. Unfortunately, this is not something you can patch in locally, because it seems that @formatjs/cli bundles everything, including TypeScript it is using.

Copy link

github-actions bot commented Sep 8, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the Stale label Sep 8, 2024
@evert
Copy link

evert commented Sep 8, 2024

Thank you creating this package. As an open source maintainer myself I know it can be a thankless task.

But bots that automatically close issues before triage is pretty hostile for end users :(

@longlho
Copy link
Member

longlho commented Nov 5, 2024

@brawaru I can't repro from the link you posted using the latest @formatjs/cli :-/ I've added an integration test as well

@brawaru
Copy link
Author

brawaru commented Nov 5, 2024

@longlho oh, seems that it's fixed by some update, then. Though, doesn't seem that the relevant code changed at all, perhaps it's just a transitive dependency update that fixed it (TypeScript?). Either way, that's great 🥳 Thank you for looking into this! ❤️ I'll close this as complete.

@brawaru brawaru closed this as completed Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants