search

gajus / table

Formats data into a string table.
Other
899 stars 77 forks source link

Restore function names #154

Closed realityking closed 3 years ago

realityking commented 3 years ago

This is a small regression from #147 where the babel plugin that adds function names to the default exports was removed. Instead of reintroducing babel just for that plug-in, I'd propose to just add the function names manually. This is mainly nice for stack traces to be more useful.

cc @nam-hle

nam-hle commented 3 years ago

Great. Sometimes it's painful to test something directly in files, I have to name the default function 🤣

realityking commented 3 years ago

Rebased to fix a merge conflict.

This change also makes the .d.ts files a lot more readable.

For example, table.d.ts currently looks like this:

import type { TableUserConfig } from './types/api';
declare const _default: (data: unknown[][], userConfig?: TableUserConfig) => string;
export default _default;

with this change it'll look like this:

import type { TableUserConfig } from './types/api';
export default function table(data: unknown[][], userConfig?: TableUserConfig): string;
coveralls commented 3 years ago

Pull Request Test Coverage Report for Build 331

Totals Coverage Status
Change from base Build 325: 0.0%
Covered Lines: 345
Relevant Lines: 345
💛 - Coveralls
gajus commented 3 years ago

What do you think about getting rid of default exports and use export const funcitonName = () => {}?

realityking commented 3 years ago

Would work for me as well, would just require changing the imports everywhere as well. Would you prefer that?

realityking commented 3 years ago

@gajus Rebased it. Would be great if you could review this soon. Fixing the merge conflicts is quite a pain as it touches a lot of code.

gajus commented 3 years ago

Yikes. Sorry, I saw this PR only after merging the other PRs.

@nam-hle Please wait on this PR to be merged before making further changes.

nam-hle commented 3 years ago

Yikes. Sorry, I saw this PR only after merging the other PRs.

@nam-hle Please wait on this PR to be merged before making further changes.

Of course. I also forgot to remind you to merge @realityking's PR. Sorry guys 😅

realityking commented 3 years ago

Urgh. Here's another shot.

gajus commented 3 years ago

Thank you @realityking @nam-hle !

gajus commented 3 years ago

:tada: This PR is included in version 6.6.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: