Skip to content

feat: Allow RegExg flags to be configured through module options - #38

Closed
Refused wants to merge 2 commits into
nuxt-community:masterfrom
Refused:master
Closed

Refused wants to merge 2 commits into
nuxt-community:masterfrom
Refused:master

Conversation

@Refused

@Refused Refused commented Apr 4, 2019 •

Copy link
Copy Markdown

Request title says it all.

export default {
  modules: [
    // Module using defaults
    '@nuxtjs/redirect-module'

    // Module using passed in RegEx flags
    ['@nuxtjs/redirect-module', { flags: 'i' }]
  ]
}

@codecov

codecov Bot commented Apr 4, 2019

Copy link
Copy Markdown

Codecov Report

Merging #38 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #38   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           2      2           
  Lines          33     33           
  Branches        8      8           
=====================================
  Hits           33     33
Impacted Files Coverage Δ
lib/module.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2dd6f01...267df30. Read the comment docs.

@TheAlexLichter

Copy link
Copy Markdown
Member

Hey 👋
Thanks for the PR!

A few things:

  1. Why having a global flags variable and not defining it locally/per regex? You can also pass in regex values like /abc/i at the moment.
  2. Docs are missing
  3. Tests are missing

@ricardogobbosouza ricardogobbosouza left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi, as @manniL said, I find it unnecessary to add this flag.
In next major release #37 we are thinking of using path-to-regexp, the same as vue-router uses to have a redirection pattern

@TheAlexLichter

Copy link
Copy Markdown
Member

Stale

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