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

Produce .d.ts (typings) and externs (closure) for use by JS #13

Closed
matanlurey opened this issue Nov 1, 2016 · 21 comments · Fixed by #1563
Closed

Produce .d.ts (typings) and externs (closure) for use by JS #13

matanlurey opened this issue Nov 1, 2016 · 21 comments · Fixed by #1563
Assignees

Comments

@matanlurey
Copy link
Contributor

No description provided.

@nex3
Copy link
Contributor

nex3 commented Nov 1, 2016

@xzyfer Does node-sass have these that we can adapt?

@adaojunior
Copy link
Contributor

@nex3
Copy link
Contributor

nex3 commented Nov 1, 2016

I'm not intimately familiar with how JS type declarations work. Is this something we should ship with the npm package? Does it need to be declared somehow?

@matanlurey
Copy link
Contributor Author

@nex3 So if you have exact same public API as node-sass, we can re-use node-sass.d.ts and perhaps publish as dart-sass.d.ts on DefiniteivelyTyped, which seems the most popular repository for these files.

@xzyfer
Copy link

xzyfer commented Nov 2, 2016

@nex3 Sass types in node-sass are defined in https://github.com/sass/node-sass/tree/master/src/sass_types.

Having them in C/C++ actually causes a decent amount of pain for distributing custom functions. Custom function packages (like eyeglass) end up with an awkward dependency on node-sass.

There is some scattered discussion on what to do about Sass types moving forward. We're currently leaning towards defining Sass types as JavaScript prototypes (or ES6 classes), distributed in a separate package.

The node-sass binding layer to LibSass will handle turning LibSass types into v8 objects. This way custom functions will only deal with JavaScript objects.

@cedx
Copy link

cedx commented Sep 5, 2018

For the folks who wonder how can we use node-sass typings with sass package...

  1. Install the node-sass typings : npm install --save-dev @types/node-sass
  2. Update the TypeScript compiler options (i.e. edit the tsconfig.json file):
{
  "compilerOptions": {
    "baseUrl": ".",
    "paths": {
      "sass": ["node_modules/@types/node-sass"]
    }
  }
}
  1. And voilà!

@binki
Copy link

binki commented Sep 5, 2018

@cedx is step 2 actually required for most people? Modern TypeScript these days automatically looks in node_modules/@types for typings.

@binki
Copy link

binki commented Sep 5, 2018

If dart already knows typings information, it would be better if it could itself automatically generate the .d.ts. That way, the typings shouldn’t get out of sync and require the community to manually update them.

@cedx
Copy link

cedx commented Sep 5, 2018

@binki Yeah, step 2 is required because without it, the TypeScript compiler will look for folder node_modules/@types/sass (and the package @types/sass does not exists).

@nex3
Copy link
Contributor

nex3 commented Sep 5, 2018

@binki

If dart already knows typings information, it would be better if it could itself automatically generate the .d.ts. That way, the typings shouldn’t get out of sync and require the community to manually update them.

Dart's notion of what the types are doesn't correspond cleanly to TypeScript's, especially since Dart Sass has to do some dirty tricks outside of Dart's type system to generate the right JavaScript API. Also, the typings already need to stay in sync between Dart Sass and Node Sass, so auto-generating them wouldn't provide much advantage.

@ghost
Copy link

ghost commented Oct 1, 2018

@cedx one option is yes to have it in the @types/folder (usually the easiest way)

However, with typescript I believe 2.3+ will pull it in from where you specify from the package.json as well. Thus editors that use typescript definition files would also be able to find them this way. VS Code, JetBrains, even Vim with the youcompleteme plugin can see these. Those are just the editors I tested off the cuff (and this is one of the big reasons to have them, honestly. The other being its much easier as a library user to grok what is happening and make sense of the API)

Here's a link to the typescript documentation on it

If the dart types were included in this repo (I couldn't find them) I think a good faith effort could be made to convert them until we figure out a better way. It can't be that far off.

@nex3
Copy link
Contributor

nex3 commented Oct 2, 2018

Again, Dart's notion of types are substantially divorced from anything TypeScript could understand. I think the best way to handle this for now would be to have a build step copy https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/node-sass into the generated npm package.

@ghost
Copy link

ghost commented Oct 5, 2018

@nex3 I completely understand.

I think the real thing here, is since we're doing it by hand anyway, is that having the Dart Types gives us a basis to go off of, with some trial and error involved, rather than guess work. I could probably reason about enough to get something put together with those types, but I don't know Dart to be able to glean it from source code. Where as the type definitions, however ever far off they may be, would at least consolidate knowledge into once place where I can simply look something up if i'm unsure and do my best to map it.

Unless you are suggesting that the node-sass API directly maps the dart-sass API?

@nex3
Copy link
Contributor

nex3 commented Oct 9, 2018

Unless you are suggesting that the node-sass API directly maps the dart-sass API?

That's right. Both JS APIs are identical.

@gluons
Copy link

gluons commented Dec 6, 2020

We shouldn't use type from node-sass.

  1. node-sass is deprecated.
    So there are no new features come to node-sass.
    node-sass type is considered outdated.
  2. node-sass don't have some option that dart-sass has. e.g. fiber option.
    So type isn't compatible.

@gluons
Copy link

gluons commented Dec 6, 2020

FYI: @types/sass seems abandoned. I have a problem with DefinitelyTyped/DefinitelyTyped#32091 for year.

@glenn2223
Copy link

glenn2223 commented May 25, 2021

Just a quick one, in regards to sass/sass#3067, as fibers seems to be dead now (ref), do I:

  1. take the type declaration from the DefinitelyTyped repo and add it straight into my file - this one
  2. ignore the property as the error can be bypassed with a @ts-ignore until the new (more preferred) API entry point emerges - which I believe doesn't use fibers??? - New JS API entrypoint sass#3056
  3. add a general property with an any type
  4. ask sass to have their npm package depend on the @types/fibers and have the /// <reference ...

@nex3
Copy link
Contributor

nex3 commented May 25, 2021

If we were going to continue using Fibers, I think the principled answer would be option 4. However, since it's dying and doesn't seem likely to be revived, I think option 3 is probably the path of least resistance.

@glenn2223
Copy link

Hey @nex3, I noticed that v1.45.0-rc.1 has now been released.

Are you planning on including a .d.ts with the npm package? This way we/I can drop the dev dependency on the already outdated/unmanaged @types/sass package. I only ask because I can see you have already done the work over at: sass/sass/js-api-doc

@nex3
Copy link
Contributor

nex3 commented Dec 2, 2021

I'll see if I can throw something together using the type definitions from the language repo.

@nex3 nex3 removed the help wanted label Dec 2, 2021
@nex3 nex3 self-assigned this Dec 2, 2021
nex3 added a commit that referenced this issue Dec 2, 2021
nex3 added a commit that referenced this issue Dec 2, 2021
@nex3 nex3 closed this as completed in #1563 Dec 2, 2021
nex3 added a commit that referenced this issue May 10, 2023
Make better use of cli_pkg
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants