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

Typing errors on 3.13.0 release #898

Closed
pierrickouw opened this issue Mar 22, 2017 · 19 comments
Closed

Typing errors on 3.13.0 release #898

pierrickouw opened this issue Mar 22, 2017 · 19 comments

Comments

@pierrickouw
Copy link

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

The last release (3.13.0) seems to have broken some type definitions.

In a .ts file compiled with tsc

import * as Raven from 'raven-js';
Raven.config(/*..*);

In a get ->

error TS2339: Property 'config' does not exist on type 'typeof ".../node_modules/raven-js/typescri...'

I see the same error in Visual Code.

What is the expected behavior?

This was working fine in 3.12.
I saw in the release page, that many changes occurred during the last release on ts files so I might not using it correctly anymore.
Thanks for your time, and do not hesitate if you need more information.

@tomasklingen
Copy link
Contributor

The typescript declaration file has changed.
The new way to import is: import Raven from "raven-js";

@pierrickouw
Copy link
Author

It's working, thanks !

@pierrickouw
Copy link
Author

Actually, I still have an issue:
import Raven from "raven-js"

// when compiled 
raven_js_1.default.config(constants_1.SENTRY_URL, {
                 ^   
TypeError: Cannot read property 'config' of undefined

@pierrickouw pierrickouw reopened this Mar 22, 2017
@michaeljones
Copy link

I have the same issue. The code appears to be doing module.exports = Raven but the typescript is using export default so they seem to be incompatible unless I am missing something? Perhaps there is a setting in typescript that allows for this? The raven typescript tests appear to use import ... from ... so perhaps it is possible to make it work?

@tomasklingen
Copy link
Contributor

tomasklingen commented Mar 22, 2017

Same. Pinned raven-js in my package.json to "raven-js": "~3.12.2" for now.

It is working in the raven-tests.ts file because it's really just testing the .d.ts file.

@benvinegar
Copy link
Contributor

benvinegar commented Mar 22, 2017

So, obviously this was caused by #827. Can anything from tsconfig.json be used in your local config?

Folks, I need a hand. I don't write TypeScript, and I rely on the community to get these definitions right.

@LucaVazz @graingert @connor4312

@benvinegar
Copy link
Contributor

benvinegar commented Mar 22, 2017

I'm guessing it's related to this:

"compilerOptions": {
  "module": "commonjs",
  // ...
}

@benvinegar
Copy link
Contributor

If I don't have a resolution by EOD I'll back out #827 entirely and publish 3.13.1.

@graingert
Copy link
Contributor

graingert commented Mar 22, 2017

the reason is your .d.ts describes an ES module, but your source code is a CJS module

@benvinegar
Copy link
Contributor

benvinegar commented Mar 22, 2017

the reason is your .d.ts describes an ES module, but your source code is a CJS module

Yep, I understand. So the "fix" is to tell the tsc to treat your entire project as CJS (module: "commonjs"), as the tests do? But I presume people can't do that for perfectly good reasons.

So is the only solution to go back to the old definitions? Can we amend the existing ones quickly to use the previous export statements?

@graingert
Copy link
Contributor

you should be able to say that Raven is a CJS module, or switch to ESM completely.

@graingert
Copy link
Contributor

graingert commented Mar 22, 2017

Options:

declare as CJS

declare module "raven-js" {
    export = Raven
}

Then move all the exported interfaces to a new .ts file (that typescript users can import) and import into raven.d.ts

declare as ESM

Then to support const Raven = require('raven-js'); add Raven back to module.exports with Raven.default === Raven and Raven.__esModule === true:

import objectAssign from 'object-assign';

const assign = Object.assign || objectAssign;
const Raven = ...
export default Raven
module.exports = assign(Raven, module.exports);

@benvinegar
Copy link
Contributor

benvinegar commented Mar 22, 2017 via email

@benvinegar
Copy link
Contributor

benvinegar commented Mar 22, 2017 via email

@graingert
Copy link
Contributor

@benvinegar I'd recommend the ESM + module.exports = Object.assign trick.

@benvinegar
Copy link
Contributor

I'm going to revert for now, publish 3.13.1, then look into getting this done for the future.

@benvinegar
Copy link
Contributor

3.13.1 is published. Keeping this open because I want to solve this TypeScript problem properly – and hopefully for the last time.

@LucaVazz
Copy link
Contributor

Ouch, I'm really sorry to have caused this trouble.

@graingert could you please open a PR with the CJS-Variant you suggested to get it fixed?

@whatisaphone
Copy link

While Typescript support is being looked at, I'd like to reflag my PR #736 from last year. Right now we're stuck on raven-js@3.2.0 because of this issue. After that version there was a regression and RavenOptions changed from exported to private.

Even in that old version, we have to import it in a very hacky way:

import { RavenOptions, RavenStatic } from 'raven-js';  // this is messy because we're waiting on a raven-js bugfix
const Raven: RavenStatic = require('raven-js');  // tslint:disable-line

Ideally with proper typings, we'd be able to do something like this:

import Raven, { RavenOptions } from 'raven-js';

export function captureMessage(msg: string, options?: RavenOptions) {
    Raven.captureMessage(msg, options);
}

My PR #736 demonstrates how this can be done.

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

No branches or pull requests

7 participants