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

Move Day.js plugin types import to global typings #2954

Merged
merged 1 commit into from
Jul 5, 2021

Conversation

lhsazevedo
Copy link
Contributor

Fixes #2417

Changes proposed in this pull request:
Remove Day.js relativeTime plugin types import from humanTime util and add it to the global typings file.
See @davwheat's proposal on #2417.

Reviewers should focus on:
Ensuring that no error occurs when compiling the humanTime util.

$ ./node_modules/.bin/tsc --esModuleInterop ./@types/global/index.d.ts ./src/common/utils/humanTime.ts

Note that you'll need to pass the --esModuleInterop to avoid the following error (unrelated to this PR):

node_modules/dayjs/index.d.ts:3:1
    3 export = dayjs;
      ~~~~~~~~~~~~~~~
    This module is declared with using 'export =', and can only be used with a default import when using the 'esModuleInterop' flag.

This flag is highly recommended by the TypeScript team for both new and existing projects. I didn't add it to tsconfig.json because I don't understand ts enough right now to guarantee that it won't break anything :)

@davwheat
Copy link
Member

davwheat commented Jul 4, 2021

This flag is highly recommended by the TypeScript team for both new and existing projects. I didn't add it to tsconfig.json because I don't understand ts enough right now to guarantee that it won't break anything :)

Great spot. I'll update the tsconfig repo now.

Actually, this flag is already part of our tsconfig repository, which core inherits from. It's weird that running tsc didn't pick it up... 🤔

@davwheat
Copy link
Member

davwheat commented Jul 4, 2021

Looks good to me.

Running ./node_modules/.bin/tsc.cmd --esModuleInterop ./src/common/utils/humanTime.ts seems to work. (Specifying the typings file in global isn't needed because it's included by the tsconfig. No errors when doing so.

@davwheat davwheat self-assigned this Jul 4, 2021
@davwheat davwheat added this to the 1.0.5 milestone Jul 4, 2021
@lhsazevedo
Copy link
Contributor Author

Great, thanks for reviewing!

@SychO9 SychO9 removed this from the 1.0.5 milestone Jul 5, 2021
Copy link
Member

@dsevillamartin dsevillamartin left a comment

Choose a reason for hiding this comment

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

Not tested locally, changes make sense though.

@SychO9 SychO9 merged commit 42dabea into flarum:master Jul 5, 2021
@lhsazevedo lhsazevedo deleted the relativetime-global-typings branch July 31, 2021 14:41
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.

Day.js plugins and Typescript
4 participants