Skip to content

Document required browserify option "dedupe: false" #93

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

kumavis
Copy link

@kumavis kumavis commented Mar 6, 2020

also:

  • some additional comments
  • example code sets { dedupe: false }
  • example code triggers dedupe
  • update browserify to version that allows setting dedupe: false

Fixes/Updates #51

@kumavis
Copy link
Author

kumavis commented Mar 6, 2020

cc @goto-bus-stop @ljharb thanks/

Copy link
Member

@goto-bus-stop goto-bus-stop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

I like that you've given better var names to some things :) Would it be possible to revert the index.js whitespace changes?

return acc;
}, {});

// Force browser-pack to wrap the common bundle
b._bpack.hasExports = true;

// for tests/debugging
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's also for plugins to hook into factored pipelines. If you have a minification step for example it can be added to the factored pipeline by using this event.

b.pipeline.get('pack').unshift(s);

if (needRecords) files = [];

next();
}));

// capture module's relative path. used for recovering entry point pathnames
Copy link
Member

@goto-bus-stop goto-bus-stop Mar 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would keep this comment but probably remove most of the others…tbh, I feel like many of the comments only restate the line and don't actually clarify much.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the comments to stay sane while trying to read through the code which i found unintuitive

Co-Authored-By: Renée Kooi <renee@kooi.me>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants