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

Refactor sandboxGlobals -> buildSandboxGlobals #245

Merged
merged 1 commit into from
Nov 1, 2019

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented Oct 31, 2019

This changes the system from providing default set of shared (and therefore mutable) global properties to using a builder function to generate the set of globals to be used per visit.

The buildSandboxGlobals function will receive the default set of globals that FastBoot creates (currently this is najax and FastBoot), and whatever the buildSandboxGlobals function returns is what will ultimately be used. If buildSandboxGlobals is not provided, a default implementation is used (it is essentially defaultGlobals => defaultGlobals;).

Closes #239

@rwjblue rwjblue added this to the v3.0.0 milestone Oct 31, 2019
@krisselden
Copy link
Contributor

We could default it to a function that does a cheap copy of the old property to ease migration.

Making it a function just helps clue people into that they should be careful here, it doesn't mitigate data leaking between visits and the existing code already assigns. This doesn't prevent interior mutations and we already Object.assign the old sandboxGlobals property.

Copy link
Contributor

@kratiahuja kratiahuja left a comment

Choose a reason for hiding this comment

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

Once Kris's concerns are addressed.

This changes the system from providing default set of shared (and
therefore mutable) global properties to using a builder function to
generate the set of globals to be used _per visit_.

The `buildSandboxGlobals` function will receive the default set of
globals that FastBoot creates (currently this is `najax` and
`FastBoot`), and whatever the `buildSandboxGlobals` function returns is
what will ultimately be used. If `buildSandboxGlobals` is not provided,
a default implementation is used (it is essentially `defaultGlobals =>
defaultGlobals;`).

For example, to specify a custom global property named `AwesomeThing` to
be accessed within the sandboxed context:

```js
let fastboot = new FastBoot({
  distPath: 'some/path/here',
  buildSandboxGlobals(globals) {
    return Object.assign({}, globals, {
      AwesomeThing: 'Taco Cat'
    });
  }
});
```

If `sandboxGlobals` is passed (and `buildSandboxGlobals` is not) issue a
deprecation and automatically create a `buildSandboxGlobals` of the
following:

```js
globals => Object.assign({}, globals, options.sandboxGlobals);
```
@rwjblue
Copy link
Member Author

rwjblue commented Nov 1, 2019

Updated to default buildSandboxGlobals from a specified options.sandboxGlobals (with a console warning). This is no longer a breaking change, I've removed the label.

@rwjblue rwjblue merged commit ed9b9d1 into ember-fastboot:master Nov 1, 2019
@rwjblue rwjblue deleted the sandbox-globals-refactor branch November 1, 2019 04:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor sandboxGlobals options into buildSandboxGlobals
3 participants