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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

When generating a new app with --embroider use all optimisation flags #10370

Merged
merged 3 commits into from
Oct 7, 2023
Merged

When generating a new app with --embroider use all optimisation flags #10370

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0039ed7
generating new app with --embroider use all flags
mansona Sep 8, 2023
ef4338c
fix fixtures
mansona Sep 18, 2023
1052c73
update embroider packages
mansona Oct 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions blueprints/app/files/ember-cli-build.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ module.exports = function (defaults) {

<% if (embroider) { %>const { Webpack } = require('@embroider/webpack');
return require('@embroider/compat').compatBuild(app, Webpack, {
staticAddonTestSupportTrees: true,
staticAddonTrees: true,
staticHelpers: true,
staticModifiers: true,
staticComponents: true,
staticEmberSource: true,
skipBabel: [
Copy link
Contributor

Choose a reason for hiding this comment

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

in my ember-cli-build, I also have these:

    // ember-inspector does not work with this flag
    // staticEmberSource: true,
    splitControllers: true,
    splitRouteClasses: true,
    packagerOptions: {
      webpackConfig: {
        // highest fidelity sourcemaps at the cost of speed
        devtool: 'source-map'
      }
    }

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, if ember-inspector does not work with that flag, I am less sure about recommending it be on by default, even with the --embroider flag

Copy link
Contributor

Choose a reason for hiding this comment

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

well, its a very new flag, and still opt in.

all the other options are about preventing userland code from regressing to be non-vite compatible (fully static)

staticEmberSource is kind of along those same lines (e.g.: "make sure you don't use the private require APIs!"),

but small progress is still progress, and I think this PR is good progress. Because there is no worse feeling than "trying the new thing", only to find out that "you weren't good enough", because the static flags weren't enabled by default.

in the afternoon RFC meeting today, we talked about a strategy for fixing the inspector in the long term for future ember-source changes (similar to what we do for supporting ember-data), and I really like that approach.

{
package: 'qunit',
Expand Down
6 changes: 3 additions & 3 deletions blueprints/app/files/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@
"<% } %>@ember/optional-features": "^2.0.0",
"@ember/string": "^3.1.1",
"@ember/test-helpers": "^3.2.0<% if (embroider) { %>",
"@embroider/compat": "^3.2.1",
"@embroider/core": "^3.2.1",
"@embroider/webpack": "^3.1.5<% } %>",
"@embroider/compat": "^3.2.3",
"@embroider/core": "^3.3.0",
"@embroider/webpack": "^3.2.0<% } %>",
"@glimmer/component": "^1.1.2",
"@glimmer/tracking": "^1.1.2<% if (typescript) { %>",
"@glint/environment-ember-loose": "^1.1.0",
Expand Down
6 changes: 3 additions & 3 deletions tests/fixtures/app/embroider-no-welcome/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@
"@ember/optional-features": "^2.0.0",
"@ember/string": "^3.1.1",
"@ember/test-helpers": "^3.2.0",
"@embroider/compat": "^3.2.1",
"@embroider/core": "^3.2.1",
"@embroider/webpack": "^3.1.5",
"@embroider/compat": "^3.2.3",
"@embroider/core": "^3.3.0",
"@embroider/webpack": "^3.2.0",
"@glimmer/component": "^1.1.2",
"@glimmer/tracking": "^1.1.2",
"broccoli-asset-rev": "^3.0.0",
Expand Down
6 changes: 3 additions & 3 deletions tests/fixtures/app/embroider-pnpm/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@
"@ember/optional-features": "^2.0.0",
"@ember/string": "^3.1.1",
"@ember/test-helpers": "^3.2.0",
"@embroider/compat": "^3.2.1",
"@embroider/core": "^3.2.1",
"@embroider/webpack": "^3.1.5",
"@embroider/compat": "^3.2.3",
"@embroider/core": "^3.3.0",
"@embroider/webpack": "^3.2.0",
"@glimmer/component": "^1.1.2",
"@glimmer/tracking": "^1.1.2",
"broccoli-asset-rev": "^3.0.0",
Expand Down
6 changes: 3 additions & 3 deletions tests/fixtures/app/embroider-yarn/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@
"@ember/optional-features": "^2.0.0",
"@ember/string": "^3.1.1",
"@ember/test-helpers": "^3.2.0",
"@embroider/compat": "^3.2.1",
"@embroider/core": "^3.2.1",
"@embroider/webpack": "^3.1.5",
"@embroider/compat": "^3.2.3",
"@embroider/core": "^3.3.0",
"@embroider/webpack": "^3.2.0",
"@glimmer/component": "^1.1.2",
"@glimmer/tracking": "^1.1.2",
"broccoli-asset-rev": "^3.0.0",
Expand Down
6 changes: 6 additions & 0 deletions tests/fixtures/app/embroider/ember-cli-build.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ module.exports = function (defaults) {

const { Webpack } = require('@embroider/webpack');
return require('@embroider/compat').compatBuild(app, Webpack, {
staticAddonTestSupportTrees: true,
staticAddonTrees: true,
staticHelpers: true,
staticModifiers: true,
staticComponents: true,
staticEmberSource: true,
skipBabel: [
{
package: 'qunit',
Expand Down
6 changes: 3 additions & 3 deletions tests/fixtures/app/embroider/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@
"@ember/optional-features": "^2.0.0",
"@ember/string": "^3.1.1",
"@ember/test-helpers": "^3.2.0",
"@embroider/compat": "^3.2.1",
"@embroider/core": "^3.2.1",
"@embroider/webpack": "^3.1.5",
"@embroider/compat": "^3.2.3",
"@embroider/core": "^3.3.0",
"@embroider/webpack": "^3.2.0",
"@glimmer/component": "^1.1.2",
"@glimmer/tracking": "^1.1.2",
"broccoli-asset-rev": "^3.0.0",
Expand Down
6 changes: 6 additions & 0 deletions tests/fixtures/app/typescript-embroider/ember-cli-build.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ module.exports = function (defaults) {

const { Webpack } = require('@embroider/webpack');
return require('@embroider/compat').compatBuild(app, Webpack, {
staticAddonTestSupportTrees: true,
staticAddonTrees: true,
staticHelpers: true,
staticModifiers: true,
staticComponents: true,
staticEmberSource: true,
skipBabel: [
{
package: 'qunit',
Expand Down
6 changes: 3 additions & 3 deletions tests/fixtures/app/typescript-embroider/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@
"@ember/optional-features": "^2.0.0",
"@ember/string": "^3.1.1",
"@ember/test-helpers": "^3.2.0",
"@embroider/compat": "^3.2.1",
"@embroider/core": "^3.2.1",
"@embroider/webpack": "^3.1.5",
"@embroider/compat": "^3.2.3",
"@embroider/core": "^3.3.0",
"@embroider/webpack": "^3.2.0",
"@glimmer/component": "^1.1.2",
"@glimmer/tracking": "^1.1.2",
"@glint/environment-ember-loose": "^1.1.0",
Expand Down