asn1js icon indicating copy to clipboard operation
asn1js copied to clipboard

Bundling with Rollup is not working

Open hodossy opened this issue 2 years ago • 5 comments

It is stated in the documentation that bundling with a standard JS bundler is straightforward, however I find it hard to make Rollup work. Could you show an example of such a configuration with any major bundler? Thanks for the efforts!

hodossy avatar Jul 12 '22 08:07 hodossy

Hey @hodossy, I also need this for Parcel as it's using rollup behind the scenes and I found the culprit of this issue and how to fix it. I will try to make a fork tomorrow with the fix and open a PR.

piotrwitek avatar Sep 08 '22 19:09 piotrwitek

This ES test works:

% cat index.mjs
import ASN1 from '@lapo/asn1js';
import Hex from '@lapo/asn1js/hex.js';
console.log(ASN1.decode(Hex.decode('06032B6570')).content());
% node index.mjs
1.3.101.112
curveEd25519
EdDSA 25519 signature algorithm

…but it does indeed not work with rollup, which complains about default not being exported.

I'm no expert of rollup unfortunately, it probably doesn't like the ES/CommonJS/AMD boilerplate I'm using.

lapo-luchini avatar Sep 09 '22 09:09 lapo-luchini

@lapo-luchini, thanks for chiming in. I believe here is the issue, and after testing, by moving "require" statements outside the conditions to the top scope it works as expected with rollup.

Avoid conditional require() ref Unlike ES module import statements which are only allowed at the top-level of a module, require is a function that may be called from anywhere. However, when require is called from within a conditional or another control flow statement, Parcel must wrap the resolved module in a function so that side effects are executed at the right time. This also applies recursively to any dependencies of the resolved module.

// 🚫 Conditional requires cause recursive wrapping if (someCondition) { require('./something'); }

piotrwitek avatar Sep 09 '22 09:09 piotrwitek

I see. Current boilerplate is used to be compatibile with CommonJS (require), and ES (import) and RequireJS at the same time (with the same code)… but seems to be a bad idea for Rollup. At the moment I don't see a solution to get the 4 options with the same code. I'm trying with @rollup/plugin-commonjs + dynamicRequireTargets but so far I didn't manage to get a working bundle.

lapo-luchini avatar Sep 09 '22 14:09 lapo-luchini

I managed to have it working with the following rollup.config.js:

import path from 'path';
import commonjs from '@rollup/plugin-commonjs';
import alias from '@rollup/plugin-alias';

export default {
    input: 'index.mjs',
    output: {
        dir: 'output',
        name: 'testRollup',
        format: 'umd',
    },
    plugins: [
        alias({
            entries: [
                { find: /@lapo[/]asn1js[/]([a-z0-9]+)(?:.js)?/, replacement: path.resolve(__dirname, 'node_modules/@lapo/asn1js/$1.js') },
                { find: '@lapo/asn1js', replacement: path.resolve(__dirname, 'node_modules/@lapo/asn1js/asn1.js') },
            ],
        }),
        commonjs({
            dynamicRequireTargets: [
                'node_modules/@lapo/asn1js/*.js',
            ]
        }),
    ],
};

I'm not sure if this is "nice enough" for common rollup usage (I never used it myself, before today).

lapo-luchini avatar Sep 09 '22 20:09 lapo-luchini

Sorry for being this late with the answer, but in the end I managed to get it working with this suggested configuration! I really appreciate the time and effort you have invested in this one, thank you!

hodossy avatar Oct 18 '22 07:10 hodossy

I'll check in the future if there's a better way. At some point I guess I'll do a 2.x release when I use native ES modules and will remove AMD boilerplate, at that point it will probably work out of the box.

lapo-luchini avatar Oct 20 '22 17:10 lapo-luchini