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

Files imported via app.import are not included in production build #6829

Closed
MartinMalinda opened this issue Mar 6, 2017 · 16 comments
Closed

Comments

@MartinMalinda
Copy link
Contributor

node: 7.5.0
os: darwin x64
ember-cli: 2.12.0-beta.1

Maybe but I'm missing something but imported files are only available in development build but not in production build.

  app.import('./bower_components/magnific-popup/dist/jquery.magnific-popup.min.js'); //not included in production build
  app.import('./vendor/plain.js'); //not included in production build

  app.import({
    production: './vendor/plain.js', //not included in production build
    development: './vendor/plain.js'
  });

Reproduction repo: https://github.com/MartinMalinda/ember-cli-import-issue

@Turbo87
Copy link
Member

Turbo87 commented Mar 7, 2017

@MartinMalinda does that happen only on the 2.12 beta or in other releases too?

@rwjblue
Copy link
Member

rwjblue commented Mar 7, 2017

Does the same issue exist when you strip the leading ./ from the paths?

@MartinMalinda
Copy link
Contributor Author

@rwjblue It works correctly without ./. It is strange that it works for dev though, It can lead to sneaky production bugs.
@Turbo87 It also does not load on 2.11, should I test on older releases as well?

I looked a few online code examples and official docs and none of them mention using ./. But still it is quite intuitive to put the ./ there as it's also supported syntax in module importing.

@stefanpenner
Copy link
Contributor

@MartinMalinda thanks to checking, that def sounds like a bug then. Although a strange one...

@stefanpenner
Copy link
Contributor

stefanpenner commented Mar 7, 2017

@rwjblue should we disallow relative paths? I am quite surprised they actually work, that isn't intended... but i can be convinced that we should support them.

@rwjblue
Copy link
Member

rwjblue commented Mar 7, 2017

@stefanpenner - I was also very surprised that relative paths worked here. IMO, relative paths "break" the mental model that we want folks to have which is that you are importing from some ephemeral tree that is reified just prior to final concatenation.

@rwjblue
Copy link
Member

rwjblue commented Mar 7, 2017

But I totally think its a bug that we somehow treat relative paths differently between ember b and ember b -prod.

@n1ru4l
Copy link

n1ru4l commented Jun 2, 2017

Edit: I was wrong :D Got it working, still struggling in understanding the ember-cli logic.

I can confirm this.

ember-cli: 2.14.0-beta.2
node: 7.9.0
os: darwin x64

I am importing a file with an absolute path in an addon. With ember build everything is okay. When using ember build -prod the file is not added to the bundle.

@pzac
Copy link

pzac commented Jul 6, 2017

I'm getting the same issue with absolute paths. Is there any workaround?

@palamike
Copy link

I use "./" (relative path) and it work perfectly for both dev and prod.

@phantomphildius
Copy link

I had this same issue as well when upgrading to ember-cli 2.12.3, removing the relative path solved it though. Thanks!

@pzac
Copy link

pzac commented Jul 14, 2017

We gave up and basically replaced App.import(foo) with import npm:foo

@Turbo87
Copy link
Member

Turbo87 commented Jul 14, 2017

FWIW the import() method was only ever designed to work with paths starting with either vendor/ or bower_components/. Everything else shouldn't have worked and shouldn't be used. Note though that starting in Ember CLI 2.15 it will also allow and work with node_modules/ too.

@Turbo87
Copy link
Member

Turbo87 commented Jul 14, 2017

I'm closing this issues as it seems to be working as intended

@Turbo87 Turbo87 closed this as completed Jul 14, 2017
@Turbo87 Turbo87 removed the bug label Jul 14, 2017
@pzac
Copy link

pzac commented Jul 14, 2017

It would be nice if the documentation made this clear, this issue drove us crazy

@Turbo87
Copy link
Member

Turbo87 commented Jul 14, 2017

@pzac pull requests welcome 😉

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

No branches or pull requests

8 participants