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

Add library index #34

Open
pronebird opened this issue Feb 5, 2019 · 10 comments
Open

Add library index #34

pronebird opened this issue Feb 5, 2019 · 10 comments

Comments

@pronebird
Copy link

pronebird commented Feb 5, 2019

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
Copy link
Owner

Why tslint doesn't like submodule imports?

@Alxandr
Copy link
Collaborator

Alxandr commented Feb 5, 2019

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
Copy link
Owner

It's not a private API in this case.

@pronebird
Copy link
Author

pronebird commented Feb 5, 2019

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
Copy link
Owner

andreypopp commented Feb 5, 2019

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
Copy link
Collaborator

Alxandr commented Feb 5, 2019

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
Copy link
Author

pronebird commented Feb 5, 2019

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
Copy link
Owner

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
Copy link
Owner

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
Copy link
Collaborator

Alxandr commented Feb 5, 2019

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.

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

3 participants