Skip to content
This repository has been archived by the owner on Dec 12, 2023. It is now read-only.
This repository has been archived by the owner on Dec 12, 2023. It is now read-only.

Content Addressable Hashes #1

Open
blakeembrey opened this issue Feb 3, 2016 · 44 comments
Open

Content Addressable Hashes #1

blakeembrey opened this issue Feb 3, 2016 · 44 comments

Comments

@blakeembrey
Copy link
Member

I'm making this issue here because things may change and I don't want to notify a lot of people ahead of time with my thoughts.

Goals:

  • De-dupe typings
  • Manifest files
  • Fix module typings conflicts

The idea:

Content addressable hashes. For the next version I'd really like to fix de-duping and re-writing of dependencies which causes issues on import still. I realised one way to solve all of the above is to use content hashes as the file names and just write wrappers to the content hashed .d.ts files. The structure will look more like:

typings/.store/klou3f3u34u982r04jdf.d.ts
typings/main/popsicle.d.ts
typings/main.d.ts
...

The stored content hashed files will be per-typed file and the nature of hashes will avoid duplicated dependencies. No more rewriting typings other than import/export still, and writing a wrapper separately. This is a pretty big architectural change though, so I'll be working on it in core first.

...

declare module 'popsicle' {
  export * from '../.store/klou3f3u34u982r04jdf.d.ts'
}
@unional
Copy link
Member

unional commented Feb 3, 2016

For my own education,

  • typings/.store/klou3f3u34u982r04jdf.d.ts is the external module definition
  • typings/main/popsicle.d.ts is the wrapper ONLY contains
declare module 'X' {
  export * from '../.store/klou3f3u34u982r04jdf.d.ts'
}

where X is specified in typings.json:

{
  "files": {
     "X": "<path to X definition (e.g. main.d.ts)>"
  }
}

or

{
  "files": [
    "<path to somename.d.ts>"
  ]
}

where X = path.slice(-5); // without '.d.ts'

@blakeembrey
Copy link
Member Author

Where X is the module name given from typings install (so option 1). However, just like currently, there'll be more than one wrapper and there'll be individual file wrappers as well (so also option 1 + option 2 - X/path/to/somename).

@unional
Copy link
Member

unional commented Feb 4, 2016

Ar, of course. It's External Module. :P

The question is we need some anchor point to determine the relative path. e.g. in the final "ambient" version (where core-js is input from typings install:

declare module 'core-js' {
  export * from '<X>';
}

declare module 'core-js/fn/array' {
  export * from '<Y>';
}

The typings.json need to have a baseline that reference to X, and a relative path X/fn/array => Y.
One possible solution is:

{
  files: {
    "main": "X",
    "main/fn/array": "Y",
    "browser": "X",
    "browser/fn/array": "Y"
  }
}

UPDATE: where X, Y are file names defined by typed file creator, and <X> <Y> is content addressable hashes file names of X and Y.

@unional
Copy link
Member

unional commented Feb 4, 2016

Another solution, reusing "main" entry point, is: expanding on the package-browser-field-spec:

{
  "main": {
    "core-js": "X",
    "./fn/array": "Y"
  },
  "browser": {
    // if user should not get the module through main entry point due to impractical file size.
    "core-js": false,
    "./fn/array": "browser-Y"
  }
}

@blakeembrey
Copy link
Member Author

I might be missing something, but what are you trying to solve with this proposal? The relative names will always come from the filename relative to typings.json, you can just put it where you want to match the actual implementation (which is what I actually do currently, if you look at each of the modules I've typed).

Aside from that, I didn't understand the "UPDATE". I don't ever want someone to actually reference the hashed files themselves, it's just an approach to fix some of the issues I'm seeing in the current Typings release.

@unional
Copy link
Member

unional commented Feb 4, 2016

Sorry, may be my English is not doing me much good. 😏

What I tried to describe is a proposal on:

  1. How it is defined in the community typed definition project (e.g. typed-core-js)
  2. How it will appear in the consuming module. (e.g. my-super-app)
    with regard to multiple export modules.

Let me try it again with an example.
core-js has multiple sub-path exports: core-js, core-js/fn/array, core-js/fn/map etc. They are relative (relate to the base core-js).
In typed-core-js, the file structure is:

+ --- main.d.ts
+ --- main.fn.array.d.ts
+ --- browser.fn.array.d.ts
+ --- typings.json

While normally main.fn.array.d.ts could be organized as main/fn/array.d.ts, we do not need to force that on the community, as long as they can do a proper mapping in typings.json they can name their files whatever they want.

In the typings.json

{
  "main": {
    "core-js": "main.d.ts",
    "./fn/array": "main.fn.array.d.ts"
  },
  "browser": {
    // if user should not get the module through main entry point due to impractical file size.
    "core-js": false,
    "./fn/array": "browser.fn.array.d.ts"
  }
}

In my-super-app, run typings install core-js --name core. Each file are downloaded and renamed based on their content:
main.d.ts -> typings/.store/hashA.d.ts
main.fn.array.d.ts -> typings/.store/hashB.d.ts
browser.fn.array.d.ts -> typings/.store/hashC.d.ts

typings/main/core-js.d.ts:

declare module 'core' {
  export * from '../.store/hashA.d.ts';
}
declare module 'core/fn/array' {
  export * from '../.store/hashB.d.ts';
}

typings/browser/core-js.d.ts:

declare module 'core/fn/array' {
  export * from '../.store/hashC.d.ts';
}

Hope it is better this time. 😏

@blakeembrey
Copy link
Member Author

@unional To be honest, I don't think it's a useful change and will restrict use in the future. Is there a valuable reason you can't replicate the file structure? I'm not convinced it's a problem that needs solving.

Edit: Just image having to maintain this with every feature going forward. Is this an override field? How does it integrate with other filesystem fields? What does main/browser point to? The original or the mapped value? Why does browser map from nice filename to real file instead of nice file to nice file or real file to real file?

@unional
Copy link
Member

unional commented Feb 4, 2016

Are you referring to this?

While normally main.fn.array.d.ts could be organized as main/fn/array.d.ts, we do not need to force that on the community, as long as they can do a proper mapping in typings.json they can name their files whatever they want

I don't see it as override field, unless you mean the browser part. But that is part of the browser-field-spec.

If we solely rely on the folder structure, how would the folder structure and typings.json look like?
I can't visualize that. :(

@blakeembrey
Copy link
Member Author

@unional It's how it already works though? E.g. https://github.com/typings/typed-es6-promise the main typing is in dist, matching the module. In https://github.com/typings/typed-debug, there's multiple entry points that also match the module. I actually type modules pretty closely with the file structure and I think the filesystem is a good place for that information to stay 😉

@blakeembrey
Copy link
Member Author

Ok, I'm going to change this issue to be about content addressable hashes and make a new issue roadmap for 0.7 (or 0.8, etc).

@blakeembrey blakeembrey changed the title Next Major Release Content Addressable Hashes Feb 4, 2016
@unional
Copy link
Member

unional commented Feb 4, 2016

Um......still not seeing it. Sorry. 😢

using the core-js example, should the file structure look like this:

+ browser
    + fn
        + array.d.ts
+ main
    + fn
        + array.d.ts
+ main.d.ts

and typings.json:

{
  "main": [
    "main.d.ts",
    "main/fn/array.d.ts"
  ],
  "browser": [
    "browser/fn/array.d.ts"
  ]
}

Or it's something else?

@unional
Copy link
Member

unional commented Feb 4, 2016

In that case how to we describe "in browser the core (main.d.ts) is not available"?
as in:
https://github.com/defunctzombie/package-browser-field-spec#ignore-a-module

@blakeembrey
Copy link
Member Author

It currently works. Using your example above, I'll refactor to the proposed model (which doesn't actually relate to the original discussion anymore).

{
  "main": "main.d.ts",
  "files": [
    "main/fn/array.d.ts"
  ],
  "browser": {
    "main/fn/array.d.ts": "browser/fn/array.d.ts"
  }
}

Main will never be multiple aliases - you can't have two "main" files on import.

@unional
Copy link
Member

unional commented Feb 4, 2016

Do you mean

{
  "main": "main.d.ts",  // <-----
  "files": [
    "main/fn/array.d.ts"
  ],
  "browser": {
    "main.d.ts": false,
    "main/fn/array.d.ts": "browser/fn/array.d.ts"
  }
}

@unional
Copy link
Member

unional commented Feb 4, 2016

Main will never be multiple aliases - you can't have two "main" files on import.

I thought typings install will get the files can create a wrapper (one single file).

@blakeembrey
Copy link
Member Author

Do you mean

Yes, typo. Updated.

will get the files can create a wrapper (one single file)

But they are aliases of multiple files with a single "main" entry. E.g. you have a bunch of declare module "..." - core-js, core-js/main/fn/array, core-js/....

@unional
Copy link
Member

unional commented Feb 4, 2016

I think I start getting what you mean. It a misconception on my side (as it should be 😏).

When I think about multiple sub-path entry, I also think of them as isolated, as they are loaded separately from other part of the module. A better example would be UI library such as ext-js or material-ui.

Since they are huge, and most application do not use every widget provided, the application will reference and load only the widget they use, e.g. material-ui/lib/grid or material-ui/lib/checkbox.

So I always think the typed file should do the same. i.e.

declare module 'material-ui/lib/grid' { export * from './lib/grid.d.ts' }
declare module 'material-ui/lib/checkbox' { export * from './lib/checkbox.d.ts' }

However, it is a typed file so it does not need to do that. It can be (syntax likely not correct):

declare module 'material-ui' {
  declare module 'lib' {
    export * from './lib/grid.d.ts'   // export function grid() {...}
    export * from './lib/checkbox.d.ts'    // export function checkbox() {...}
  }
}

Simply because there is no taxing on loading (at least in tsc).
But there would be taxing on loading for module loader that loads TypeScript directly onto browser and transpile there (i.e. jspm). This taxing only exists during development as in production the code will be transpiled to js and bundled.

@unional
Copy link
Member

unional commented Mar 6, 2016

Do you have plan on when this will go in? It's definitely a great idea. Currently we still have module name pollution:

declare module 'assertion-error/main' { ... }
declare module 'assertion-error' {
  export * from 'assertion-error/main';
}
// consuming.ts
import {Assertion} from 'assertion-error';
import {Assertion} from 'assertion-error/main'; // pollution

😏

@blakeembrey
Copy link
Member Author

@unional I don't quite follow. The top level declarations are entirely intentional. You don't want them?

@blakeembrey
Copy link
Member Author

It should be mirroring the actual module: https://github.com/chaijs/assertion-error/blob/master/package.json#L19. The idea is to expose the interfaces realistically and allowed "deep requires" - however frowned upon sometimes.

@unional
Copy link
Member

unional commented Mar 6, 2016

Ar, I mean the second one.
another example:

declare module 'chai~assertion-error/main' { ... }

@unional
Copy link
Member

unional commented Mar 6, 2016

It should be mirroring the actual module: https://github.com/chaijs/assertion-error/blob/master/package.json#L19. The idea is to expose the interfaces realistically and allowed "deep requires" - however frowned upon sometimes.

I see, so at least for the package.json/main, you can indeed do this?

import {Assertion} from 'assertion-error/main`;

I thought npm dereference them by resolving the relative path. i.e.:

import abc = require('assertion-error/a/b/c/d/abc`);
// resolves to `node-modules/assertion-error/a/b/c/d/abc`

@unional
Copy link
Member

unional commented Mar 6, 2016

Your first comment:

declare module 'X' {
  export * from '../.store/klou3f3u34u982r04jdf.d.ts'
}

completely avoid this issue because there are no extra declare module 'X~something'

@blakeembrey
Copy link
Member Author

Oh, yeah. Sadly, there'll always be pollution until things are native in TypeScript. However, did you see #26? I still have a bit more work now thinking about it, but it's creating an extra name that'll be harder to run into.

The export * is not allow by TypeScript though, I tried it. It was more a nice if it was supported.

@blakeembrey
Copy link
Member Author

You can do import {Assertion} from 'assertion-error/index' - /main would not be found because it's not in the module.

@unional
Copy link
Member

unional commented Mar 6, 2016

Um... in npm-chai I break up the typings into separate files explicitly for this purpose.
The original typed file in DT has the pollution (not on declare module 'abc', but declare namespace abc).

That's why I thought the syntax used here can avoid pollution.

@blakeembrey
Copy link
Member Author

Doesn't the npm-chai repo represent the real module structure? That's what I normally try to follow I suppose we can have a flag in typings.json that avoids exposing this information easily enough. Want to make an issue with a proposal?

@unional
Copy link
Member

unional commented Mar 6, 2016

I still think the pollution is avoidable. Let me try it out first.

@unional
Copy link
Member

unional commented Mar 6, 2016

Seems like the issue about npm-chai is gone!

Now all the extra ChaiStatic interfaces does not show up anymore.

Likely you have already fix it somehow. 🌹

@blakeembrey
Copy link
Member Author

Gone? Since when? I'm yet to merge all this new stuff, have to refactor the CLI in typings/typings.

@unional
Copy link
Member

unional commented Mar 6, 2016

Probably sometime between 0.6.8 to 0.6.10.

@unional
Copy link
Member

unional commented Mar 6, 2016

You can do import {Assertion} from 'assertion-error/index'

Does it mean that typings.json/main should match source package main field?
i.e. if the source has package.json/main: "lib/abc", the typings should also has typings.json/main: "lib/abc.d.ts"?

@blakeembrey
Copy link
Member Author

Ideally, yes. It would be nice, but not required - not many people "deep require". On the failure, I don't think much changed between those versions except for supporting TS 1.8 module augmentation - is that related?

@unional
Copy link
Member

unional commented Mar 6, 2016

Not sure, or may even because of TS upgrade from 1.8.2 to 1.8.7.

Will file issue if similar things pop up so I have a concrete case to present (instead of relying on memory, which can easily be corrupted. 😛 )

@unional
Copy link
Member

unional commented Apr 4, 2016

As I understand, this is blocked because we can't:

declare namespace abc {
  import main = require('~abc');
  export = main;

or something similar.
Currently this is a limitation on TypeScript.

Is there an issue opened on https://github.com/Microsoft/TypeScript can be referenced and tracked here?

@blakeembrey
Copy link
Member Author

It can technically still be done using the name hacks we currently do, but I'd like to remove all the tilde hacks and use this instead. It's not tracked, I figure it's a feature limitation they have on purpose.

@blakeembrey
Copy link
Member Author

Perhaps I should look around and open an issue just in case anyway?

@unional
Copy link
Member

unional commented Apr 4, 2016

Yeah, because that means almost all typings need to be written twice. One for module one for script tag.

@blakeembrey
Copy link
Member Author

Yeah, because that means almost all typings need to be written twice. One for module one for script tag.

Can you elaborate? This should not change anything behaviour-wise.

@unional
Copy link
Member

unional commented Apr 4, 2016

It can technically still be done using the name hacks we currently do

Um......actually I don't get this part....

@unional
Copy link
Member

unional commented Apr 4, 2016

@blakeembrey
Copy link
Member Author

Um......actually I don't get this part....

We can go declare module '~<content-hash>' as a hack.

This won't fix your ambient/non-ambient change. The only tweak I'd like to have is re-exporting from an external module in an ambient declaration.

@unional
Copy link
Member

unional commented Apr 4, 2016

ar..I see.
Maybe I should open another issue regarding ambient/non-ambient.

@unional
Copy link
Member

unional commented Apr 4, 2016

Created: typings/typings#402

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants