Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Wrong type definition for placeholder type property #921

Closed
1 of 4 tasks
lukevella opened this issue Mar 22, 2021 · 7 comments
Closed
1 of 4 tasks

Wrong type definition for placeholder type property #921

lukevella opened this issue Mar 22, 2021 · 7 comments
Assignees
Milestone

Comments

@lukevella
Copy link

Category

  • Enhancement
  • Bug
  • Question
  • Documentation gap/issue

Version

Please specify what version of the library you are using: [3.4.0]

Desired Behavior

The following code should work fine:

const pptx = new PptxGenJs()

pptx.defineSlideMaster({
    title: "PLACEHOLDER_SLIDE",
    objects: [
        {
            placeholder: {
                options: { name: "body", type: "body", x: 0.6, y: 1.5, w: 12, h: 5.25 },
                text: "(custom placeholder text!)",
            },
        },
    ],
});

Observed Behavior

We get the following typescript error:

Screenshot 2021-03-22 at 14 58 22

I observed that we can satisfy typescript by passing PptxGenJs.PLACEHOLDER_TYPES.body as type however this causes a runtime error. Seems like the type definition for PlaceholderProps.type is just wrong.

Steps to Reproduce

Copy and paste the snippet above in to a typeescript project.

@jmroon
Copy link

jmroon commented Mar 22, 2021

I'm having similar issues.

import PptxGenJS from "pptxgenjs";

I can create a new instance:

var pptx = new PptxGenJS();

But even though my IDE thinks I can access the namespaced enums, it seems as though PptxGenJs only allows me to access its constructor and not the namespaced enums as follows:

{
  placeholder: {
    text: "PLACEHOLDER TITLE",
    options: {
      name: "title",
      type: PptxGenJS.PLACEHOLDER_TYPES.title,
      x: 0,
      y: 0,
      w: "25%",
      h: "10%",
    },
  },
}

This compiles correctly with no complaints from TS or ESLint, but I get a runtime error saying PptxGenJS is undefined, which is odd since I figured that should resolve at compile time and not runtime.

Perhaps this is because the exported class and namespace share the same name?

@jmroon
Copy link

jmroon commented Mar 22, 2021

This is not ideal, but you can at least bypass the issue with the following:

type: "title" as pptxgen.PLACEHOLDER_TYPES

@jmroon
Copy link

jmroon commented Mar 23, 2021

This is the result from the transpilation:

type: pptxgenjs__WEBPACK_IMPORTED_MODULE_1__[\"default\"].PLACEHOLDER_TYPES.title

Looks like maybe only the default export gets imported, and the namespace gets ignored.

I should add that my compiler target is esnext. Haven't tried another version yet.

gitbrent added a commit that referenced this issue Mar 24, 2021
@gitbrent gitbrent self-assigned this Mar 24, 2021
@gitbrent gitbrent added this to the 3.5.0 milestone Mar 24, 2021
gitbrent added a commit that referenced this issue Mar 24, 2021
@gitbrent
Copy link
Owner

@lukevella - Good catch. The type needs to be exposed correctly like ChartType etc.

Fixed in head:
Screen Shot 2021-03-24 at 00 14 43

Now it can be used without issues:
Screen Shot 2021-03-24 at 00 12 14

@gitbrent
Copy link
Owner

gitbrent commented Mar 24, 2021

@lukevella @jmroon - you're welcome to test the fix by patching your node_modules/pptxgenjs/types/index.d.ts - just add this as shown above on line 29

readonly PlaceholderType: typeof PptxGenJS.PLACEHOLDER_TYPES

@lukevella
Copy link
Author

@gitbrent That change does not seem to fix the issue for me. What does fix it is the following change:

export interface PlaceholderProps {
  name: string
- type: PLACEHOLDER_TYPES
+ type: keyof typeof PLACEHOLDER_TYPES
  x: Coord
  y: Coord
  w: Coord
  h: Coord
}

@gitbrent
Copy link
Owner

@lukevella - okay, there's yet another change needed - we're using type in other defs, but an enum here, hence the issue.

Added:

    export type PLACEHOLDER_TYPE = 'title' | 'body' | 'pic' | 'chart' | 'tbl' | 'media'

Updated:

    export interface PlaceholderProps {
        name: string
        type: PLACEHOLDER_TYPE
        x: Coord
        y: Coord
        w: Coord
        h: Coord
    }

gitbrent added a commit that referenced this issue Mar 27, 2021
gitbrent added a commit that referenced this issue Mar 31, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

3 participants