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

feat(commonjs): Expose cjs detection and support offline caching #604

Merged
merged 1 commit into from Oct 15, 2020

Conversation

lukastaegert
Copy link
Member

@lukastaegert lukastaegert commented Oct 9, 2020

Rollup Plugin Name: commonjs

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • no

If yes, then include "BREAKING CHANGES:" in the first commit message body, followed by a description of what is breaking.

List any relevant issue numbers:

Description

Note: This features relies on the yet-to-be-released rollup/rollup#3813, specifically the new moduleParsed hook. Once that is released and the peer dependency version of this plugin has been adjusted, tests should be green and this PR will be ready for merge.

This will expose the result of the CommonJS file-type detection as a module meta property, exposed via this.getModuleInfo(id).meta.commonjs.isCommonJS or the module info passed to the moduleParsed hook.

This will for the first time allow this plugin to work with caching data taken from a disk cache that was not created in the same session. Previously in such a setup, Rollup would hang as the file type detection promises would never resolve. Now you can do this and it will work:

const { readFileSync, writeFileSync } = require('sander');
const rollup = require('rollup');
const cjs = require('@rollup/plugin-commonjs');

rollup
  .rollup({
    input: 'src/main',
    cache: getCache(),
    plugins: [cjs()]
  })
  .then(bundle => {
    storeCache(bundle.cache);
    return bundle.write({
      format: 'es',
      dir: 'dist'
    });
  });

function getCache() {
  try {
    return JSON.parse(readFileSync(__dirname, 'cache.json'));
  } catch (e) {
    return null;
  }
}

function storeCache(cache) {
  try {
    writeFileSync(__dirname, 'cache.json', JSON.stringify(cache));
  } catch (e) {
    console.log('Could not store cache', e);
  }
}

Depending on this, I seriously consider adding some built-in disk-cache capabilities into rollup-cli now (probably via a small detour across zlib to save disk space).

@lukastaegert lukastaegert marked this pull request as draft October 9, 2020 15:07
@lukastaegert
Copy link
Member Author

cc @underfin

@lukastaegert lukastaegert marked this pull request as ready for review October 13, 2020 18:16
@lukastaegert
Copy link
Member Author

As the relevant Rollup version has been released, this is now ready for review! Note that this is a breaking change as it raises the peerDependency version of Rollup.

@LarsDenBakker LarsDenBakker self-assigned this Oct 14, 2020
Copy link
Contributor

@LarsDenBakker LarsDenBakker left a comment

Choose a reason for hiding this comment

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

LGTM!

@shellscape
Copy link
Collaborator

@lukastaegert a recent merged caused a few conflicts. Please take a peek

BREAKING CHANGES: Raises minimal required Rollup version to 2.30.0
@lukastaegert
Copy link
Member Author

Done. Also made sure to add the BREAKING CHANGES marker to the first commit.

@shellscape shellscape merged commit 70fad01 into master Oct 15, 2020
@shellscape shellscape deleted the commonjs-expose-cjs branch October 15, 2020 15:47
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

Successfully merging this pull request may close these issues.

None yet

3 participants