indutny / asn1.js

ASN.1 Decoder/Encoder/DSL
MIT License
181 stars 64 forks source link

Hidden cyclical dependency in base #98

Open Vanuan opened 6 years ago

Vanuan commented 6 years ago

There's this base file which contains a hidden cyclical dependency. The problem is buffer depends on base while base depends on buffer.

https://github.com/indutny/asn1.js/blob/bbf14e0732b7d8a5addedb800c91da7bc2a17e83/lib/asn1/base/index.js#L1-L8

https://github.com/indutny/asn1.js/blob/master/lib/asn1/base/buffer.js#L4 It works in node (and probably in webpack), but it causes problems when deployed using https://unpkg.com

Vanuan commented 6 years ago

To resolve this dependency cycle, inner modules should depend on inner modules directly, without index.js as intermediate:

// index.js
const buffer = require('./buffer');
const reporter = require('./reporter');
const Node = require('./node');

module.exports = {
  Reporter: reporter.Reporter,
  DecodeBuffer: buffer.DecodeBuffer,
  EncoderBuffer: buffer.EncoderBuffer,
  Node,
};
// buffer.js
const Reporter = require('./reporter').Reporter;

Are there any reasons this wasn't done this way?

balthazar commented 5 years ago

I agree, this cause cycle requires in some environments which could potentially be error-prone

lordvlad commented 5 years ago

Rollup is another example environment where this breaks.