search

andreypopp / validated

Validate your configurations with precise error messages
91 stars 14 forks source link

Add library index #34

Open pronebird opened 5 years ago

pronebird commented 5 years ago

Hi,

I've just stumbled upon the lint error emitted by tslint with very much default configuration:

ERROR: 30:8 no-submodule-imports Submodule import paths from this package are disallowed; import from the root instead

Would it be possible to define the index.js file and re-export everything from there?

andreypopp commented 5 years ago

Why tslint doesn't like submodule imports?

Alxandr commented 5 years ago

Cause submodules are in general thought to be private implementation details. At least that's my understanding of it. 

        rationale: Lint.Utils.dedent`
            Submodules of some packages are treated as private APIs and the import
            paths may change without deprecation periods. It's best to stick with
            top-level package exports.`,
andreypopp commented 5 years ago

It's not a private API in this case.

pronebird commented 5 years ago

My understanding is that basically anyone can import any file within the package which is gonna work , but it may break if the file wasn't somehow announced as public and was moved between releases.

However, each package has an entry file (i.e: index.js) which is somewhat guaranteed to be available.

andreypopp commented 5 years ago

I think tslint is generalizing too much here and I don't believe in usefulness of such rules. One can easily use API in a non supported way anyway or rely on some undocumented behaviour. This is JavaScript after all.

The motivation behind splitting validated API into multiple top level modules is that those modules are optional — one can use only schema and other can validate data coming from JSON5 and yet another — from runtime JSON representation. Having all in the same module means that one have to bundle JSON5 parser even if they don't need it.

Alxandr commented 5 years ago

Yeah. I don't necessarily disagree with you @andreypopp. It's a bit of a tricky issue IMHO. I started working on a prototype v3 a few months back that used a mono-repo approach (so you have validated-json5 instead of validated/json5, but I never got very far with it.

pronebird commented 5 years ago

I think tslint is generalizing too much here and I don't believe in usefulness of such rules. One can easily use API in a non supported way anyway or rely on some undocumented behaviour. This is JavaScript after all.

It's possible that you're right. I'd say they exist to guard junior developers from making mistakes.

The motivation behind splitting validated API into multiple top level modules is that those modules are optional — one can use only schema and other can validate data coming from JSON5 and yet another — from runtime JSON representation. Having all in the same module means that one have to bundle JSON5 parser even if they don't need it.

You mean just because index.js re-exports the JSON5 parser, it would have to be included in the bundle, regardless if it's used or not outside of the module?

andreypopp commented 5 years ago

You mean just because index.js re-exports the JSON5 parser, it would have to be included in the bundle, regardless if it's used or not outside of the module?

Correct.

andreypopp commented 5 years ago

Yeah. I don't necessarily disagree with you @andreypopp. It's a bit of a tricky issue IMHO. I started working on a prototype v3 a few months back that used a mono-repo approach (so you have validated-json5 instead of validated/json5, but I never got very far with it.

Yeah, splitting into multiple packages will solve this issue.

Alxandr commented 5 years ago

You mean just because index.js re-exports the JSON5 parser, it would have to be included in the bundle, regardless if it's used or not outside of the module?

Correct.

Correct with an asterisk. Tree shaking/minification would generally get rid of it if it's unused, but it's not guaranteed.