Include RAILS_RELATIVE_URL_ROOT environment variable in publicPath.

[jpickwell]

Fix #1357.

[gauravtiwari]

Could you add some test for this, please? using different paths

[jpickwell]

I thought about that, but wasn't sure which test file to put them in? I was thinking of using the config test file, but wasn't sure because I didn't see anything in there for publicPath.

[gauravtiwari]

Ahh yes, but could you please add one :)

/* global test expect, describe */

const { chdirTestApp, chdirCwd } = require('../utils/helpers')

chdirTestApp()

describe('Config', () => {
  beforeEach(() => jest.resetModules())
  afterAll(chdirCwd)

  test('public path', () => {
    delete process.env.RAILS_RELATIVE_URL_ROOT
    const config = require('../config')
    expect(config.publicPath).toEqual('/packs/')
  })

  test('public path with relative root', () => {
    process.env.RAILS_RELATIVE_URL_ROOT = '/foo'
    const config = require('../config')
    expect(config.publicPath).toEqual('/foo/packs/')
  })

  test('should return extensions as listed in app config', () => {
    const config = require('../config')
    expect(config.extensions).toEqual([
      '.js',
      '.sass',
      '.scss',
      '.css',
      '.module.sass',
      '.module.scss',
      '.module.css',
      '.png',
      '.svg',
      '.gif',
      '.jpeg',
      '.jpg'
    ])
  })
})

[jpickwell]

Will do. Thanks for the boilerplate.

[gauravtiwari]

Great thanks 👍

[gauravtiwari]

I guess we need to set RAILS_ENV in the test for consistent results:

 test('public path', () => {
    process.env.RAILS_ENV = 'development'
    delete process.env.RAILS_RELATIVE_URL_ROOT
    const config = require('../config')
    expect(config.publicPath).toEqual('/packs/')
  })

  test('public path with relative root', () => {
    process.env.RAILS_ENV = 'development'
    process.env.RAILS_RELATIVE_URL_ROOT = '/foo'
    const config = require('../config')
    expect(config.publicPath).toEqual('/foo/packs/')
  })

[jpickwell]

I assume you meant NODE_ENV in your examples.

[jpickwell]

Nope, you meant RAILS_ENV.

[gauravtiwari]

oh yes! I guess you figured it out 😉

[jpickwell]

The [^:]\/ part doesn't seem correct because if public_output_path contains a protocol, then publicPath would have a leading slash; e.g., (ignoring relative root) public_output_path='https://cdn.com' and publicPath='/https://cdn.com/'. The protocol would be preserved with the current RegExp, but that doesn't appear to be correct. Maybe I'm missing something.

[gauravtiwari]

Yeah, but at the moment Rails handles the CDN side of things so it's fine but you are right this needs more check if we expect the public_output_path to have a protocol as well (which we don't at the moment).

[jpickwell]

Sounds good.