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

[1.3] - How to prevent TypeScript from marking the meteor/PACKAGE_NAME imports as error? #6177

Closed
dotansimha opened this issue Feb 6, 2016 · 19 comments

Comments

@dotansimha
Copy link
Contributor

Hi,

I am using 1.3-beta.8 and I try to import Atmosphere packages into my code.
So in order to import them, I use:

import {MyExport} from `meteor/MYPACKAGE`;

It works great, but there is only one thing that bugging me...
I am also use TypeScript as my compiler, and TypeScript marks that line as error, because it's an unknown import - it looks for a Node package named meteor inside the node_modules folder, and there is not such directory, so the IDE marks that import as error.

The error appears in TypeScript compiler log as unknown import, and marked as error in the IDE (WebStorm11).

I guess that if Meteor can create a fake NPM package named meteor under the node_modules and create a fake definitions file under that directory, it will solve that issue.

@dotansimha dotansimha changed the title [1.3] - How to prevent IDE from setting the meteor/PACKAGE_NAME imports as error with TypeScript? [1.3] - How to prevent IDE from marking the meteor/PACKAGE_NAME imports as error with TypeScript? Feb 6, 2016
@dotansimha dotansimha changed the title [1.3] - How to prevent IDE from marking the meteor/PACKAGE_NAME imports as error with TypeScript? [1.3] - How to prevent TypeScript from marking the meteor/PACKAGE_NAME imports as error? Feb 6, 2016
@stubailo
Copy link
Contributor

stubailo commented Feb 8, 2016

I guess that if Meteor can create a fake NPM package named meteor under the node_modules and create a fake definitions file under that directory, it will solve that issue.

This would be awesome because I also want to use ESLint to detect incorrect imports!

@stubailo stubailo added this to the Release 1.3 milestone Feb 8, 2016
@jamiter
Copy link
Contributor

jamiter commented Feb 8, 2016

And this would also solve the same issue with Flow static type checking. Maybe a separate issue, but also absolute imports do not work for Flow. Do they work in TypeScript?

@stubailo
Copy link
Contributor

stubailo commented Feb 8, 2016

Maybe a separate issue, but also absolute imports do not work for Flow.

Doesn't facebook have some weird system that prevents them from writing down the import paths at all? I always see these annotations with @providesModule at the top of the files. Is that a Flow thing, or some other tool?

@avital
Copy link
Contributor

avital commented Feb 9, 2016

Does anyone want to build a pull request for this?

@jamiter
Copy link
Contributor

jamiter commented Feb 11, 2016

@stubailo, I didn't know about the @providesModule yet and it took some digging around to find more information. This comment sheds some light on how they implemented it. Basically it will first look for a @providesModule statement and then look into the node_modules directory to find stuff. So in theory, if the meteor/thing module is available in node_modules, most of it should be solved. Except for absolute paths, because they're not supported in flow at all, if I'm correct.

So fixing this for TypeScript should fix it for Flow as well.

@avital, about the PR: to be honest, I have no idea where to start.

P.S. I was trying out flow because it is available in the ecmascript package and didn't require me to change all js files into ts files to get static type checking. Flow being available made me think/hope it was supported out of the box, but without the import/export syntax in 1.2 it feels quite useless. Haven't seen much discussion about it either. Would be another great selling point if Meteor 1.3 would support this out of the box. It's very close at doing so.

@raix
Copy link
Contributor

raix commented Feb 11, 2016

@avital do you want a pr updating the build tool to generate the node_modules/meteor package?

@avital
Copy link
Contributor

avital commented Feb 11, 2016

What do IDEs show when people use Webpack-style requires that include non-standard characters like !? They probably also mark those as errors, no?

I personally think the answer of "just have the linter ignore those lines via whatever nolint directive" is OK

As for what the pull request would look like, honestly, I don't know.

@jamiter
Copy link
Contributor

jamiter commented Feb 20, 2016

Just a heads up for people using Flow and encountered the absolute path issue. You can add a module.name_mapper in your .flowconfig so Flow will be able to resolve them:

[options]
module.name_mapper='^\/\(.*\)$' -> '<PROJECT_ROOT>/\1'

Now Flow understands your absolutes paths!

EDIT:
To solve the meteor/PACKAGE import issue for Flow you can add this to your .flowconfig:

module.name_mapper='^meteor\/\(.*\)$' -> '<PROJECT_ROOT>/.meteor/local/build/programs/server/packages/\1'

If Meteor moves its packages to NPM, this will no longer be an issue, right?

@stubailo
Copy link
Contributor

Wow this is awesome info!

@bySabi
Copy link

bySabi commented Feb 20, 2016

It seens that Typescript module path mapping on tsconfig.json is on next Typescript version 2.0.

microsoft/TypeScript#5039
microsoft/TypeScript#5728

tsconfig.json will be, Ex: (don`t tested my self)

{ 
  "compilerOptions": {
    "sourceMap": true,
    "target": "es6",
    "module": "commonjs",
    "jsx": "react",
    "experimentalDecorators": true,
    "baseUrl": ".",
    "paths": { "meteor/*": ["node_modules/*"] }
  }
}

@tomitrescak
Copy link

@bySabi will this really solve the problem for Typescript? The meteor packages do not reside in node_modules directory and they do not conain typescript definitions so nothing will be resolved. Isn't the only way to rewrite the meteor.d.ts file to contain all type declarations for meteor packages? And for other packages as well.

Although paths work, following does not:

"baseUrl": ".",
"paths": {
      "meteor/*": [
        ".meteor/local/build/programs/server/packages/*"
      ]
}

[EDIT]

Modifying Meteor.d.ts does the trick and forces to correctly specify all import paths. For example, the Accounts package is wrapped into module as following:

declare module "meteor/accounts-base" {
  export module Accounts {
  ...
  }
}

I will be rewriting this and if anyone will be interested I can post the full .d.ts file later or even submi it to DefinitelyTyped as version 1.3.

Thanks

@tomitrescak
Copy link

The initial version for Meteor 1.3 can be found here: https://gist.github.com/tomitrescak/8366ce98f1857e202ea8

@tomitrescak
Copy link

I am not sure where Session belongs ... I think it is part of "meteor/meteor" will experiment with that later.

@bySabi
Copy link

bySabi commented Mar 3, 2016

@tomitrescak I don´t tested my self. Only found the PR on Typescript repo and I thoungh it could be the answer to import on meteor.

Is on my schedule a meteor-typescriptnpm fork to test it. Let me know is you make a progress about and we can join forces.

In my schedule I have too a webpack:typescript implementation for @eXon package. I don´t know if he write some code. Do you write it, @eXon ?

@tomitrescak
Copy link

Well, I am successfully using the file from that gist in my Mantra application. Also, I wrote several other typings for flow-router and more. Will put it online soon. I think, @eXon is working on the webpack:typescript.

@omeid
Copy link
Contributor

omeid commented Mar 31, 2016

@tomitrescak Thanks for the very helpful gist, any progress with uploading your work? Hopefully you have aldeed:collection2 sorted as well, otherwise, I am happy to contribute.

@tomitrescak
Copy link

tomitrescak commented May 1, 2016

@omeid I do have a lot of the packages done, but it's all just about wrapping the existing package d.ts into extrenal module definition

declare module "meteor/aldeed:collection2" {
   // original stuff
}

I'm exploring now the amazing work of Blake Rembley https://github.com/typings/typings and am thinking of adding typings there.

[EDIT] I will not be adding typings to Blake's site and I hope that meteor MDG will roll out their own set. I simply do not have the time. If anyone wants to do this I can provide latest Ambient definition for Meteor.

@hwillson
Copy link
Contributor

The Guide's current recommendation for using TypeScript with Meteor is to use the barbatus:typescript package. See that projects README for instructions on how to install Meteor specific typings, that will address this issue. Closing - thanks!

@wildhart
Copy link
Contributor

@hwillson, the barbatus:typescript package is no longer the recommended approach, and the original problem of auto-importing type definition files for 3rd party packages still exists. Possible dupe of #10828.

Forum discussion: https://forums.meteor.com/t/atmosphere-packages-with-typescript-definitions/54292

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