Skip to content

Documentation for deprecated options #1309

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

Closed
defnorep opened this issue May 23, 2018 · 6 comments
Closed

Documentation for deprecated options #1309

defnorep opened this issue May 23, 2018 · 6 comments

Comments

@defnorep
Copy link

defnorep commented May 23, 2018

What problem does this feature solve?

My issue doesn't fit neatly within either Bug Report or Feature Request -- it's likely closest to Feature Request as I am requesting more information in the documentation. Please see the Proposed API section for my proposed resolution to this matter.

It doesn't seem to be clear from the migration documentation how we are supposed to migrate usage of postLoaders to a Webpack Module Rule, specifically for the default template SFC block. In Vue Loader 14, we were able to assign a Webpack loader to the html property of postLoaders to obtain access to the emitted render function. The documentation does indicate how to process a new type of template language (pug), but not how to work with the render function that is emitted from a default template block.

In my case, I parse the emitted render function with an AST to extract parameters of specific function calls instead of statically parsing file contents using gettext. I have attempted to load the output from a template block in a number of different ways, referring to the Webpack documentation as well as the Vue Loader documentation. My loader is able to parse the script block but not the render function emitted from the template. Should we be targeting test with /\.html$/, or /\.vue$/? Do we need to use a resourceQuery? Do we need to use enforce: 'post'? Do we need to add a new language extension to every template block? There are a lot of variables in this that the standard Webpack documentation cannot address easily. I suppose this is one of the challenges of being coupled to Webpack -- their documentation needs to be complemented.

I seem to have the most success with this rule:

    {
      resourceQuery: /^\?vue/,
      enforce: 'post',
      loader: resolve(__dirname, '../translations/webpackLoader'),
    },

Though it seems bare:

Starting new source: /[redacted]/modules/common/components/SampleComponent/index.vue
var render = function () {}
var staticRenderFns = []

export { render, staticRenderFns }

Component template for reference:

<template>
  <div>
    <h2>{{ label }}</h2>
    <p>This is dummy text</p>
    <p>{{ $t('Sample Component for !{name}', 'This is the context', { name }) }}</p>
  </div>
</template>

What does the proposed API look like?

If there is an obvious reason for not being able to parse the template block's render function, I can create a PR to call it out in the docs right away. If there is not an obvious reason, I will spend some time creating a reproduction repository and file a bug report so we can investigate further and then add our findings (examples and explanations) to the docs.

@defnorep defnorep changed the title Documentation for deprecated options incomplete Documentation for deprecated options May 23, 2018
@yyx990803
Copy link
Member

That's an interesting use case and TBH I never thought about this when designing v15. There are two aspects of the problem:

  1. v15 doesn't assume a lang for the default <template> blocks. Maybe we need to give it a default lang="html" so it's consistent with v14.

  2. v15 injects the template compiler in the pitcher, after all matched loaders. Unfortunately webpack does not preserve enforce: 'post' information in loaderContext.loaders, so currently there's no way for vue-loader to know that the template compiler needs to be injected before your loader.

@defnorep
Copy link
Author

defnorep commented May 24, 2018

Hey Evan, thank you for replying. My knowledge of Webpack and vue-loader internals is limited so please bear with me.

I agree with point number one.

For point number two, if we cannot rely on Webpack's capabilities, then I wonder if we are looking at bringing back postLoaders as postTemplateLoaders or something similarly named. Looks like we still have access to the options in this scope. The concept of a Webpack loader is fairly first-class in vue-loader so it doesn't feel like it leaks, though it would be an artifact from your previous design decisions. Ideally we could remove it in favour of Webpack options later, assuming we can get an upstream change.

If we decide that adding an option compromises design decisions, or that we have to wait for a possible upstream change, I can solve my problem by moving all of my translation call sites to computed properties. Some of them have to be there anyways for binding within HTML attributes or using v-html. The vast majority of our translations are within our templates however so it would be a fairly large change for us, and we like the API the way that it is.

Again, thanks a lot for your attention on this. I'm willing to contribute a PR depending on what we decide to do.

@defnorep
Copy link
Author

Submitted a PR, as linked above.

I went the route of adding an option called templatePostLoaders. It fixes my use case.

@zgayjjf
Copy link

zgayjjf commented May 29, 2018

I didn't realize it that postLoaders & preLoaders & loaders have been Deprecated...
I think these options for loaders in V14 are better than it's replacement in V15, for they were clear & consistent.
While apis in V15 are not that clear, and break consistence——you have to add another option to bring back functionality like this #1328 , which makes configuration separated and breaks the consistence.

Yet, in spite of this, #1328 is a good way to make this right. I would be glad if it is merged.
@Daekano Thank you for your work.

@defnorep
Copy link
Author

@zgayjjf You're welcome!

I think the idea of leveraging standard Webpack Rules to post process explicit content types is a good design goal. It actually increased consistency, until we found this edge case on the render functions. Hopefully we can one day remove this option.

@davidguilherme
Copy link

@yyx990803 I just opened a merge request with an alternative solution for this issue without bringing the postLoader option back.

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 a pull request may close this issue.

4 participants