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

allow splittable user to use additional plugins in the deps finding pipeline #51

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

Conversation

erwinmombay
Copy link

this is needed as there are some constructs that the jison library
generates that blows up with "use strict" directive. I need to be able
to pass "transform-remove-strict-mode" plugin

…ipeline

this is needed as there are some constructs that the jison library
generates that blows up with "use strict" directive. I need to be able
to pass "transform-remove-strict-mode" plugin
splittable.js Outdated
@@ -238,9 +238,10 @@ exports.getGraph = function(entryModules, config) {
// directly and which we don't want to apply during deps finding.
transform(babel, {
babelrc: false,
plugins: [
plugins: Array.isArray(config.babel.plugins) ? config.babel.plugins.concat([
Copy link
Owner

Choose a reason for hiding this comment

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

Should this be a new config option or the same as above?

Copy link
Author

Choose a reason for hiding this comment

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

ideally i think it should be a different one. since the presets list used in the transform phase could be different in the deps finding phase.

Copy link
Owner

Choose a reason for hiding this comment

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

Lets rename then.

Copy link
Author

Choose a reason for hiding this comment

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

should we allow for 2 babel configs or should a hacky config.babel.depSearchPhasePlugins do?

Copy link
Owner

Choose a reason for hiding this comment

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

Can we maybe make the plugin you need default?

Copy link
Author

@erwinmombay erwinmombay Sep 25, 2017

Choose a reason for hiding this comment

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

ideally it shouldn't since the "use strict" directive is desirable after all (correct me if im wrong), its just unfortunate that jison uses a lot of labels and jumps

@cramforce
Copy link
Owner

cramforce commented Sep 25, 2017 via email

@erwinmombay
Copy link
Author

@cramforce ah right. that makes sense. ill make the changes and move the export babel plugin back into the splittable repo

visitor: {
Program(path, state) {
const id = path.scope.generateUidIdentifier('uid')
const constNumDecl = t.variableDeclaration('const', [
Copy link
Author

Choose a reason for hiding this comment

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

@jridgewell is this the right way to add export someModuleUniqueId = 42;? would appreciate advice

Copy link

@jridgewell jridgewell Oct 4, 2017

Choose a reason for hiding this comment

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

Almost. Drop the t.exportSpecifier:

export default function (babel) {
  const { types: t } = babel;
  
  return {
    visitor: {
      Program(path) {
        const uid = path.scope.generateUidIdentifier('uid');
        const declar = t.variableDeclaration('const', [
          t.variableDeclarator(uid, t.numericLiteral(42)),
        ]);
        
        const ex = t.exportNamedDeclaration(declar, []);
        path.unshiftContainer("body", ex);
      }
    }
  };
}

http://astexplorer.net/#/gist/9d4d2830b0b2b476627117b6a7798929/e97ac0bf49dd086154e1fd3c3657d90bf330d55c

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.

3 participants