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

Allow fullPaths to be configurable from the consuming app #108

Closed
wants to merge 1 commit into from
Closed

Allow fullPaths to be configurable from the consuming app #108

wants to merge 1 commit into from

Conversation

timiyay
Copy link

@timiyay timiyay commented Sep 18, 2016

This PR solves the lack of configurability of fullPaths to support other production-like deployment environments, like staging.

See #107

@nathanhammond
Copy link
Collaborator

This doesn't account for the fact that app isn't guaranteed to have a project property, engines, and nested addon use. The entire approach to browserify is about to change, see #100.

I'm closing this PR because I don't want to try to land any other changes before the reimagining is complete but I'm leaving open the issue. Sound reasonable?

@timiyay
Copy link
Author

timiyay commented Sep 18, 2016

@nathanhammond yeah fair enough, I charged headlong into the naive fix (time from GH issue to PR = 8 mins), so I'm happy to defer completely to the maintainers.

My issue sprung up around a potentially-related problem, and configuring fullPaths may not be the fix anyway. For background, I'm trying to switch to ember-browserify, but my app dies if it has been built and deployed by Travis (rather than locally).

The crashes occur when mapbox-gl-js - which I require as an npm:mapbox-gl-js import - tries to create web workers. See mapbox/mapbox-gl-js#3123 (comment) if curious.

@stefanpenner
Copy link
Collaborator

is there a reason why don't just always specify fullPaths to false?

@ef4
Copy link
Owner

ef4 commented Sep 19, 2016

Yes, it's because browserify doesn't correctly do incremental rebuilds when fullPaths is false. It's a performance optimization for working in development.

This is why we only default fullPaths: true for non-production builds.

@timiyay
Copy link
Author

timiyay commented Sep 19, 2016

Perhaps we could take the reverse approach, hardcoding fullPaths: true for development and test?

This would support arbitrary environments with fullPath: false, such production, staging, qa, etc

@ef4
Copy link
Owner

ef4 commented Sep 19, 2016

@timiyay that is a misunderstanding of how ember-cli handles environment. The word is unfortunately confusing.

qa, staging, and production environments are all production builds as far as ember-cli is concerned. But they can be different deploy targets (in ember-cli-deploy parlance).

Attempting to extend ember-cli's list of environments beyond development/test/production is not supported and likely to break.

@timiyay
Copy link
Author

timiyay commented Sep 19, 2016

Ah righto, I'll need to review our environment / deploy practices for supporting a staging environment.

Our setup is based on a late-2015 recommendation from ember-cli-deploy, on creating a new Ember CLI environment for staging. It appears this recommendation no longer appears in the ember-cli-deploy docs.

@timiyay timiyay deleted the configurable-full-paths branch September 18, 2019 23:10
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.

4 participants