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

Remove najax from default set of sandbox globals. #247

Merged
merged 1 commit into from
Nov 1, 2019

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented Oct 31, 2019

For applications that still need najax (for example, if they are using
an older ember-data version), they can do (example uses new
buildSandboxGlobals API):

const najax = require('najax');

let fastboot = new FastBoot({
  distPath: 'some/path/to/dist',
  buildSandboxGlobals(globals) {
    return Object.assign({}, globals, {
      najax,
    });
  }
});

See #246 for more details about this change.

Closes #246

* Recent versions of ember-data (3.12+) will automatically use
  `ember-fetch` (or global `fetch`) if present, and does not need
  special logic around `najax`
* New applications (as of ember-cli@3.13.0) include `ember-fetch` by
  default, and do **not** include jQuery by default. Considering that
  `najax` is a `jQuery.ajax` emulation API, I think this library should
  avoid exposing it.
* `najax` (as a dependency of this library) is difficult for the host
  application to control (e.g. get their own version), and would be
  better if they provided it themselves (via `buildSandboxGlobals` API)
* Providing backwards compatibility is **very** easy (add `najax:
  require('najax')` via `buildSandboxGlobals`)
* It seems better to "follow a spec" (suggesting `fetch` usage)

For applications that still need `najax` (for example, if they are using
an older ember-data version), they can do (example uses new
`buildSandboxGlobals` API):

```js
const najax = require('najax');

let fastboot = new FastBoot({
  distPath: 'some/path/to/dist',
  buildSandboxGlobals(globals) {
    return Object.assign({}, globals, {
      najax,
    });
  }
});
```
@rwjblue rwjblue merged commit b8e8690 into ember-fastboot:master Nov 1, 2019
@rwjblue rwjblue deleted the remove-najax branch November 1, 2019 04:16
@runspired
Copy link

fwiw you need najax even in current versions of ember-data if you don't have ember-fetch installed. This change trolled our fastboot tests in ember-data itself because when we test the jquery scenario we add @ember/jquery and remove ember-fetch, once we bump to fastboot 3 this fails (see embermap/ember-cli-fastboot-testing#471 and ember-fastboot/ember-cli-fastboot#829).

It's not clear how to add najax back in from this example for ember-cli-fastboot-testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Drop najax global by default?
2 participants