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

support empty rootURL with Ember CLI >= 5.0 #196

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jelhan
Copy link

@jelhan jelhan commented Oct 17, 2023

Until Ember CLI v5 the baseURL was set to '/' if rootURL was an empty string. Since Ember CLI v5 the baseURL is not set anymore on the config.options object at all. This change causes ember-cli-live-reload to break if applications set baseURL = '' in the config/environment.js.

TypeError: Cannot read properties of undefined (reading 'replace')
    at Class.serverMiddleware (/path/to/code/node_modules/ember-cli-inject-live-reload/lib/index.js:62:38)
    at ExpressServerTask.<anonymous> (/path/to/code/node_modules/ember-cli/lib/tasks/server/express-server.js:107:24)
    at promiseMapSeries (/path/to/code/node_modules/ember-cli/node_modules/promise-map-series/index.js:7:24)
    at async ExpressServerTask.startHttpServer (/path/to/code/node_modules/ember-cli/lib/tasks/server/express-server.js:258:5)
    at async ServeTask.run (/path/to/code/node_modules/ember-cli/lib/tasks/serve.js:96:5)
    at async Class.run (/path/to/code/node_modules/ember-cli/lib/commands/serve.js:106:5)
    at async /path/to/code/node_modules/ember-cli/lib/cli/cli.js:173:32

Nullish coalescing operator should solve the issue as it does not fall back to baseURL if rootURL is an empty string.

Please find a simple reproduction here: https://github.com/jelhan/ember-app-with-relative-root-url ember serve is working fine for the last commit using Ember CLI v4.12. If you go back one commit, which uses Ember CLI v5.0 it breaks with the error mentioned above.

@jelhan jelhan changed the title support empty rootURL with Ember CLI >= 5.0 WIP: support empty rootURL with Ember CLI >= 5.0 Oct 17, 2023
Comment on lines +51 to +56
let baseURL = options.liveReloadBaseUrl ?? options.rootURL ?? options.baseURL;

// express requires absolute paths
if (baseURL === '') {
baseURL = '/';
}
Copy link
Author

Choose a reason for hiding this comment

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

As an alternative pattern we could fall back to '/' not only if baseURL is an empty string but always when it is falsy. This would match previous behavior of Ember CLI more closely as options.baseURL had such a fallback.

Suggested change
let baseURL = options.liveReloadBaseUrl ?? options.rootURL ?? options.baseURL;
// express requires absolute paths
if (baseURL === '') {
baseURL = '/';
}
let baseURL = options.liveReloadBaseUrl || options.rootURL || options.baseURL || '/';

I'm not much opinionated on either or option.

@jelhan jelhan changed the title WIP: support empty rootURL with Ember CLI >= 5.0 support empty rootURL with Ember CLI >= 5.0 Oct 17, 2023
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.

1 participant