adopted-ember-addons / ember-cli-hot-loader

An early look at what hot reloading might be like in the ember ecosystem
MIT License
99 stars 13 forks source link

require.resolve fallback #89

Closed shamcode closed 6 years ago

shamcode commented 6 years ago

PR fo fix #88

toranb commented 6 years ago

@shamcode is it possible we can try path.relative(app.project.root, require.resolve()) first, then fallback to a simple require.resolve if the compiler is not truly available? I ask because back in July I heard from a few community members who bumped into a problem using require.resolve from their mono repo so we altered it and it solved a problem for those developers at the time

https://github.com/toranb/ember-cli-hot-loader/commit/fb0a91124d200480d62af5c164e03ab62a19d162

shamcode commented 6 years ago

@toranb, this changed PR, now require.resolve works like a fallback. I think that this is exactly what is described in this comment and that ember-cli do when call app.import

toranb commented 6 years ago

One issue I found testing this manually with a bigger mono repo just now

    var npmCompilerPath = path.join('ember-source', 'dist', 'ember-template-compiler.js');
    var resolvedPath = require.resolve(npmCompilerPath);
    var npmPath = path.relative(this.project.root, resolvedPath);

The npmPath returns ../node_modules/ember-source/dist/ember-template-compiler.js because the node_modules directory is a symlink (pointing back to the parent folder to reuse/share node_modules installed at the top most level of the mono repo). I'm curious if you can think of any way to solve this given I've got a local node_modules folder... yet I get the ../node_modules path

shamcode commented 6 years ago

Perhaps the solution will be to add the addon options templateCompilerPath, which will be used as the path to the ember-template-compiler.js. If this option is not specified, then use the default approach:

path.relative(app.project.root, require.resolve(npmCompilerPath));

Adding this option will preserve backward compatibility and add the ability to use this awesome addon in other specific environments. What do you think about it?

toranb commented 6 years ago

ah I'm digging it actually - so configure it if you need to override the default sorta thing. Where would you override this exactly? command line, config env or something else altogether ?

shamcode commented 6 years ago

I think config env will be more familiar to the community. For example, like this:

//my-app/config/environment.js
ENV['ember-cli-hot-loader'] = {
  templateCompilerPath: require.resolve('ember-source/dist/ember-template-compiler.js')
}
toranb commented 6 years ago

@shamcode I'm down for that - you willing to update the PR in that case? I can test it manually in a few apps I know work today to ensure backwards compat

shamcode commented 6 years ago

PR updated

toranb commented 6 years ago

Awesome! I'll give this a look later today! Thanks again

toranb commented 6 years ago

@shamcode I had success using this in a mono repo (with the configuration below) and for existing hot reload apps I tested it was backwards compatible :)

I've published this on npm and it's available now using v1.2.0

  if (environment === 'development') {
    ENV['ember-cli-hot-loader'] = {
      templateCompilerPath: 'node_modules/ember-source/dist/ember-template-compiler.js'
    }
  }

I'll expand the README later next week to include an example for others to better grok this extension point. Thanks again for taking the time to unlock this configuration option!