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

implement a feature of defaultLang(#497) #500

Closed
wants to merge 5 commits into from
Closed

implement a feature of defaultLang(#497) #500

wants to merge 5 commits into from

Conversation

JounQin
Copy link

@JounQin JounQin commented Nov 26, 2016

No description provided.

Copy link
Member

@kazupon kazupon left a comment

Choose a reason for hiding this comment

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

LGTM!

/ping @vuejs/vue-loader-vueify
I think that is useful.
What do you think?

@blake-newman
Copy link
Member

Hmm I think specifying Lang attributes is useful, especially for large projects.

A new starter to a project may not be aware of the toolset in use. So specifying the Lang attribute in the code can help them navigate to a correct resource.

Only a minor thought.

@JounQin
Copy link
Author

JounQin commented Jan 14, 2017

@blake-newman then just don't use this feature, it's not required.

@barraponto
Copy link

barraponto commented Feb 7, 2017

i don't understand why you need a default language option.
right now you can explicitly tell what language you're using in the template, and also how to deal with any "language". notice that i can make up any value as i need: i can use <style lang="australian-css"> and then configure vue-loader to pass australian-css through a specific postcss configuration.

@JounQin
Copy link
Author

JounQin commented Feb 7, 2017

If I'm using stylus in every .vue file, with this feature we don't need to write lang="styl" everywhere, same with pug as temple.

Please notice that, it's not required, if you don't like it, you can just don't use it.

@JounQin
Copy link
Author

JounQin commented Feb 7, 2017

Of course, you also could overwrite the language with the original syntax <style lang="australian-css">.

@barraponto
Copy link

In your stylus everywhere scenario, wouldn't you be able to set stylus-loader as the default loader for css just by setting the vue-loader options? As in http://vue-loader.vuejs.org/en/configurations/advanced.html

module.exports = {
  // other options...
  module: {
    // module.rules is the same as module.loaders in 1.x
    rules: [
      {
        test: /\.vue$/,
        loader: 'vue-loader',
        options: {
          // `loaders` will overwrite the default loaders.
          // The following config will cause all <style> tags without "lang"
          // attribute to be loaded with stylus-loader
          loaders: {
            css: 'stylus-loader'
          },
        }
      }
    ]
  }
}

@JounQin
Copy link
Author

JounQin commented Feb 8, 2017

The option loaders is not designed for it, and if the default pre-processor is incompatible with css syntax (for example sass with ident syntax), then in you solution, we will not be able to override the lang(<style lang="css"></style>) to use plain css anymore.

But with the defaultLang feature, you still can use plain css syntax with sass.

@octref
Copy link
Member

octref commented Feb 25, 2017

Please don't.

This makes it impossible to write textmate grammar that syntax-highlights embedded code correctly.

https://github.com/vuejs/vue-syntax-highlight will break for whoever that's changing default lang. All users of Sublime / Atom / VSCode will be affected.

@JounQin
Copy link
Author

JounQin commented Feb 26, 2017

@octref OMG, it is just an optional feature, OK? You don't need it? Just don't use it.

@octref
Copy link
Member

octref commented Feb 26, 2017

I agree with @blake-newman in that this adds implicitness which makes code less readable. As scss/less/postcss are syntactically similar, with this feature you can write them all in <style>, and without going to the setting you wouldn't know which one is being used.

This also strongly couples code with config, so a piece of vue SFC code can't be used in another project if they don't have the same setting.

And as @barraponto pointed out, if you really want to do it you can change the config yourself. Just beaware the syntax highlighting will break, and it's impossible to change the grammar to support this feature.

@barraponto
Copy link

octref OMG, it is just an optional feature, OK? You don't need it? Just don't use it.

I guess that's not the point. It is, of course, up to the maintainer to choose whether to merge or not your PR. Yet features do not come for free -- they require tests, documentation and maintenance. And as this feature has its own effects on code legibility, it's worth weighing them out.

Open source means you can add all the features you want to your versions (forks) of any piece of code. Yet it doesn't mean every PR should be merged upstream. Try not to take the outcome of this PR personally.

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.

5 participants