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

"use *.tag.html extension" rule #6

Merged
merged 1 commit into from
Feb 22, 2016
Merged

Conversation

jbmoelker
Copy link
Member

Summary

Proposed changes:

  • add "use *.tag.html extension" rule (incl. summary, why and how).

Checklist

  • The changes only affect or contain one style guide rule or section.
  • The changes are in style guide format.
  • The new style guide section has an entry in the Table of Contents (only if changes introduce new section).
  • The changes can be merged into the target branch without conflicts.

petergoes added a commit that referenced this pull request Feb 22, 2016
@petergoes petergoes merged commit 18a6516 into master Feb 22, 2016
@petergoes
Copy link
Contributor

Completely agree

@jbmoelker jbmoelker deleted the use-tag-html-extension branch February 22, 2016 15:36
@adrianblynch
Copy link

adrianblynch commented Jun 30, 2016

I'm very much keeping this in mind, Opinionated RiotJS Style Guide..., but having a .tag.html extension when requiring tags on the server causes issues.

Works:

const riot = require('riot')
const tag = require('my.tag')

Fails:

const riot = require('riot')
const tag = require('my.tag.html')

When you require riot, it makes require aware of files with the .tag extension. If you change it to .tag.html, require doesn't know how to parse it and throws an error.

@jbmoelker
Copy link
Member Author

jbmoelker commented Jul 4, 2016

@adrianblynch good point. I had never tried to import a tag directly. Are you using the actual file basename? Or is it a path (./path/to/my.tag)? Or is it actually a compiled tag (my.tag.js)?

Is this setup anywhere in the Riot docs? I'm wondering if Riot also has a way to configure the extension in this setup, since it does in the compiler. So that we could do something like this:

const riot = require('riot')
riot.settings.ext = 'tag.html' // don't know if this exists, looking for something it
const tag = require('my.tag')

Otherwise I think we should add a "browser only" note to this rule in this guide.

@jbmoelker
Copy link
Member Author

@adrianblynch any chance you are using Webpack with a specific loader? Like this:

    module: {
        loaders: [
            {
                test: /\.tag$/,
                loader: "tag",
                exclude: path.resolve(__dirname, "node_modules")
            }
        ]
    }

So you could change this to test: /\.tag.html$/:

    module: {
        loaders: [
            {
                test: /\.tag.html$/,
                loader: "tag",
                exclude: path.resolve(__dirname, "node_modules")
            }
        ]
    }

@adrianblynch
Copy link

Hi @jbmoelker - We are using webpack but in this case it was requiring in tags, const tag = require('my.tag.html' that was causing an issue. We went back to .tag. So no biggie.

jbmoelker added a commit that referenced this pull request Jul 16, 2016
jbmoelker added a commit that referenced this pull request Aug 30, 2016
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