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] Add shouldParseScope, disableTemplateTag, disableFunctionCall options #333

Merged
merged 1 commit into from
Feb 22, 2021

Conversation

pzuraq
Copy link

@pzuraq pzuraq commented Feb 20, 2021

This PR refactors modules to enable users to pass an options, with
three options currently supported:

  1. shouldParseScope, which allows users to parse the scope parameter
    into a static format usable by the precompiler

    {
      modules: {
        'ember-cli-htmlbars': 'default',
        '@ember/template-compilation': {
          export: 'precompileTemplate',
          shouldParseScope: true,
        }
      }
    }

    When enabled, the scope parameter is parsed, and then turned into an
    array of the keys on the object. If a non-object is passed, or any of
    the keys or values are not references, then an error is thrown.

  2. disableTemplateTag, which disables using the precompile macro as a
    template tag. This should be used for precompileTemplate in
    ember-cli-htmlbars

  3. disableFunctionCall, which disables using the precompile macro as
    a function call. This should be used by experimental template import
    syntaxes.

This PR also refactors the way that imports statements are processed.
They're now parsed in the beginning, in Program, ensuring that the
parse step is only done once, and that we can build a list of all
present imports in the file. This allows us to support more than one
module at once.

@pzuraq pzuraq changed the title [FEAT] Add shouldParseScope and disableTemplateTag options [FEAT] Add shouldParseScope, disableTemplateTag, disableFunctionCall options Feb 20, 2021
Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

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

Looks pretty good, thanks for working on it!

A few thoughts:

  • I think adding the flexibility to disallowing one type of invocation or the other, but I don't really want to leverage it 😝
  • Moving the ImportDeclaration hook to Program means that we can no longer detect imports that are created from other plugins (because Program will not be called again). This is already an issue with the Ember global import code we just landed (we'll have to fix this up in both babel plugins that have that code), we should be careful not to introduce even more of these race conditions.
  • I think we should add @ember/template-compilation to the default values for the module. This is probably fine to add in a follow up PR though.

__tests__/tests.js Show resolved Hide resolved
index.js Outdated
if (!importDefaultSpecifier) {
let input = state.file.code;
let usedImportStatement = input.slice(node.start, node.end);
let msg = `Only \`import ${first.local.name} from '${module}'\` is supported. You used: \`${usedImportStatement}\``;
Copy link
Member

Choose a reason for hiding this comment

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

This seems vaguely odd, why do we reference first here? Can you update to either have the new modules format suggest a local name when the export is default (I think this option, but is pretty "legacy" IMO since we are generally going to use non-default exports going forward).

index.js Show resolved Hide resolved
index.js Show resolved Hide resolved
index.js Show resolved Hide resolved
@pzuraq pzuraq force-pushed the add-precompile-template branch 2 times, most recently from 24ba661 to e483a98 Compare February 22, 2021 17:48
This PR refactors `modules` to enable users to pass an options, with
three options currently supported:

1. `shouldParseScope`, which allows users to parse the `scope` parameter
   into a static format usable by the precompiler

    ```js
    {
      modules: {
        'ember-cli-htmlbars': 'default',
        '@ember/template-compilation': {
          export: 'precompileTemplate',
          shouldParseScope: true,
        }
      }
    }
    ```

    When enabled, the scope parameter is parsed, and then turned into an
    array of the keys on the object. If a non-object is passed, or any of
    the keys or values are not references, then an error is thrown.

2. `disableTemplateLiteral`, which disables using the precompile macro as a
   template tag. This should be used for `precompileTemplate` in
   `ember-cli-htmlbars`

3. `disableFunctionCall`, which disables using the precompile macro as
   a function call. This should be used by experimental template import
   syntaxes.

This PR also refactors the way that imports statements are processed.
They're now parsed in the beginning, in `Program`, ensuring that the
parse step is only done once, and that we can build a list of all
present imports in the file. This allows us to support more than one
module at once.

wip
@rwjblue rwjblue merged commit 95d9738 into master Feb 22, 2021
@rwjblue rwjblue deleted the add-precompile-template branch February 22, 2021 17:58
@rwjblue
Copy link
Member

rwjblue commented Feb 22, 2021

Chatted with @pzuraq about the import ordering issue, and going to fix that in a follow on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants