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

Inconsistent builds with addons having files with same names #724

Closed
lifeart opened this issue May 16, 2019 · 22 comments
Closed

Inconsistent builds with addons having files with same names #724

lifeart opened this issue May 16, 2019 · 22 comments

Comments

@lifeart
Copy link

lifeart commented May 16, 2019

reproduction repo: https://github.com/lifeart/ts-merging-issue

how to reproduce?
yarn; ember test; ember test; ember test
or see

WARNING: ts-app has added the class-properties plugin to its build, but ember-cli-babel provides these by default now! You can remove the transforms, or the addon that provided them, such as @ember-decorators/babel-transforms.
WARNING: ts-app has added the class-properties plugin to its build, but ember-cli-babel provides these by default now! You can remove the transforms, or the addon that provided them, such as @ember-decorators/babel-transforms.
Environment: test
cleaning up...
Built project successfully. Stored in "C:\Users\lifeart\Documents\repos\tmp\ts-app\tmp\class-tests_dist-UBMhIWw3.tmp".
ok 1 Chrome 74.0 - [2 ms] - ESLint | app: app/models/meta-model.js
ok 2 Chrome 74.0 - [0 ms] - ESLint | app: app.js
ok 3 Chrome 74.0 - [1 ms] - ESLint | app: resolver.js
ok 4 Chrome 74.0 - [0 ms] - ESLint | app: router.js
ok 5 Chrome 74.0 - [0 ms] - TemplateLint: ts-app/templates/application.hbs
ok 6 Chrome 74.0 - [0 ms] - ESLint | tests: test-helper.js
ok 7 Chrome 74.0 - [108 ms] - Unit | Model | base model: it exists
ok 8 Chrome 74.0 - [25 ms] - Unit | Model | meta model: it exists
ok 9 Chrome 74.0 - [0 ms] - ember-qunit: Ember.onerror validation: Ember.onerror is functioning properly

1..9
# tests 9
# pass  9
# skip  0
# fail  0

# ok
WARNING: ts-app has added the class-properties plugin to its build, but ember-cli-babel provides these by default now! You can remove the transforms, or the addon that provided them, such as @ember-decorators/babel-transforms.
WARNING: ts-app has added the class-properties plugin to its build, but ember-cli-babel provides these by default now! You can remove the transforms, or the addon that provided them, such as @ember-decorators/babel-transforms.
Environment: test
cleaning up...
Built project successfully. Stored in "C:\Users\lifeart\Documents\repos\tmp\ts-app\tmp\class-tests_dist-s6n8AZfw.tmp".
ok 1 Chrome 74.0 - [2 ms] - ESLint | app: app/models/meta-model.js
ok 2 Chrome 74.0 - [0 ms] - ESLint | app: app.js
ok 3 Chrome 74.0 - [0 ms] - ESLint | app: resolver.js
ok 4 Chrome 74.0 - [0 ms] - ESLint | app: router.js
ok 5 Chrome 74.0 - [1 ms] - TemplateLint: ts-app/templates/application.hbs
ok 6 Chrome 74.0 - [0 ms] - ESLint | tests: test-helper.js
ok 7 Chrome 74.0 - [106 ms] - Unit | Model | base model: it exists
not ok 8 Chrome 74.0 - [26 ms] - Unit | Model | meta model: it exists
    ---
        expected: >
            addon-meta-model/baseModel
        stack: >
                at Object.<anonymous> (http://localhost:7357/assets/tests.js:67:14)
                at runTest (http://localhost:7357/assets/test-support.js:5618:30)
                at Test.run (http://localhost:7357/assets/test-support.js:5604:6)
                at http://localhost:7357/assets/test-support.js:5831:12
                at processTaskQueue (http://localhost:7357/assets/test-support.js:5197:24)
                at advanceTaskQueue (http://localhost:7357/assets/test-support.js:5182:4)
        negative: >
            false
        browser log: |
    ...
ok 9 Chrome 74.0 - [0 ms] - ember-qunit: Ember.onerror validation: Ember.onerror is functioning properly

1..9
# tests 9
# pass  8
# skip  0
# fail  1

same tests failing randomly

ember-cli: 3.10.0
node: 10.15.1
os: win32 x64

case:

ts addon, having model, named meta-model,
ts app has base-model and meta-model models.

app.base-model extends addon.meta-model
app.meta-model extends app.base-model

So, sometimes (randomly) app.base-model not extends from addon.meta-model and it's produce inconsistent build

@lifeart
Copy link
Author

lifeart commented May 16, 2019

This behavour may be related to ember-cli/ember-ajax#428

@chriskrycho
Copy link
Member

  1. Looks like your repro is missing a couple files!
  2. Just checking; is this a result of having .ts files in the addon's app directory?

@lifeart
Copy link
Author

lifeart commented May 16, 2019

@chriskrycho 2 - no, 1 - updated.

@lifeart
Copy link
Author

lifeart commented May 16, 2019

our production case:

yarn workspaces:
shared-addon/addon/models/meta-model.ts, shared-addon/addon/app/models/meta-model.js
dreamcatcher-app/app/models/{base-model,meta-model}.ts

@lifeart lifeart changed the title Inconsistents builds with addons having files with same names Inconsistent builds with addons having files with same names May 16, 2019
@lifeart
Copy link
Author

lifeart commented May 16, 2019

if it's hard to get fails:

  • clean system temp folder
  • try to change cpu speed
  • run ember test && ember test && ember test && ember test && ember test && ember test && ember test && ember test && ember test && ember test && ember test && ember test && ember test && ember test && ember test && ember test && ember test
$ ember test && ember test && ember test && ember test && ember test && ember test && ember test && ember test && ember test && ember test && ember test && ember test && ember test && ember test && ember test && ember test && ember test
- Building
WARNING: ts-app has added the class-properties plugin to its build, but ember-cli-babel provides these by default now! You can remove the transforms, or the addon that provided them, such as @ember-decorators/babel-transforms.
- Building
WARNING: ts-app has added the class-properties plugin to its build, but ember-cli-babel provides these by default now! You can remove the transforms, or the addon that provided them, such as @ember-decorators/babel-transforms.
- Building
Environment: test
- Building
- cleaning up
cleaning up...
- cleaning up
Built project successfully. Stored in "C:\Users\lifeart\Documents\repos\tmp\ts-app\tmp\class-tests_dist-xBzQD8hz.tmp".
ok 1 Chrome 74.0 - [1 ms] - ESLint | app: app/models/meta-model.js
ok 2 Chrome 74.0 - [0 ms] - ESLint | app: app.js
ok 3 Chrome 74.0 - [0 ms] - ESLint | app: resolver.js
ok 4 Chrome 74.0 - [1 ms] - ESLint | app: router.js
ok 5 Chrome 74.0 - [0 ms] - TemplateLint: ts-app/templates/application.hbs
ok 6 Chrome 74.0 - [0 ms] - ESLint | tests: test-helper.js
ok 7 Chrome 74.0 - [103 ms] - Unit | Model | base model: it exists
ok 8 Chrome 74.0 - [26 ms] - Unit | Model | meta model: it exists
ok 9 Chrome 74.0 - [1 ms] - ember-qunit: Ember.onerror validation: Ember.onerror is functioning properly

1..9
# tests 9
# pass  9
# skip  0
# fail  0

# ok
- Building
WARNING: ts-app has added the class-properties plugin to its build, but ember-cli-babel provides these by default now! You can remove the transforms, or the addon that provided them, such as @ember-decorators/babel-transforms.
- Building
WARNING: ts-app has added the class-properties plugin to its build, but ember-cli-babel provides these by default now! You can remove the transforms, or the addon that provided them, such as @ember-decorators/babel-transforms.
- Building
Environment: test
- Building
- cleaning up
cleaning up...
- cleaning up
Built project successfully. Stored in "C:\Users\lifeart\Documents\repos\tmp\ts-app\tmp\class-tests_dist-JLgkeu9e.tmp".
ok 1 Chrome 74.0 - [1 ms] - ESLint | app: app/models/meta-model.js
ok 2 Chrome 74.0 - [1 ms] - ESLint | app: app.js
ok 3 Chrome 74.0 - [0 ms] - ESLint | app: resolver.js
ok 4 Chrome 74.0 - [0 ms] - ESLint | app: router.js
ok 5 Chrome 74.0 - [0 ms] - TemplateLint: ts-app/templates/application.hbs
ok 6 Chrome 74.0 - [0 ms] - ESLint | tests: test-helper.js
ok 7 Chrome 74.0 - [104 ms] - Unit | Model | base model: it exists
ok 8 Chrome 74.0 - [25 ms] - Unit | Model | meta model: it exists
ok 9 Chrome 74.0 - [1 ms] - ember-qunit: Ember.onerror validation: Ember.onerror is functioning properly

1..9
# tests 9
# pass  9
# skip  0
# fail  0

# ok
- Building
WARNING: ts-app has added the class-properties plugin to its build, but ember-cli-babel provides these by default now! You can remove the transforms, or the addon that provided them, such as @ember-decorators/babel-transforms.
- Building
WARNING: ts-app has added the class-properties plugin to its build, but ember-cli-babel provides these by default now! You can remove the transforms, or the addon that provided them, such as @ember-decorators/babel-transforms.
- Building
Environment: test
- Building
- cleaning up
cleaning up...
- cleaning up
Built project successfully. Stored in "C:\Users\lifeart\Documents\repos\tmp\ts-app\tmp\class-tests_dist-gGCDNhcl.tmp".
ok 1 Chrome 74.0 - [1 ms] - ESLint | app: app/models/meta-model.js
ok 2 Chrome 74.0 - [1 ms] - ESLint | app: app.js
ok 3 Chrome 74.0 - [0 ms] - ESLint | app: resolver.js
ok 4 Chrome 74.0 - [0 ms] - ESLint | app: router.js
ok 5 Chrome 74.0 - [0 ms] - TemplateLint: ts-app/templates/application.hbs
ok 6 Chrome 74.0 - [0 ms] - ESLint | tests: test-helper.js
ok 7 Chrome 74.0 - [106 ms] - Unit | Model | base model: it exists
ok 8 Chrome 74.0 - [24 ms] - Unit | Model | meta model: it exists
ok 9 Chrome 74.0 - [0 ms] - ember-qunit: Ember.onerror validation: Ember.onerror is functioning properly

1..9
# tests 9
# pass  9
# skip  0
# fail  0

# ok
- Building
WARNING: ts-app has added the class-properties plugin to its build, but ember-cli-babel provides these by default now! You can remove the transforms, or the addon that provided them, such as @ember-decorators/babel-transforms.
- Building
WARNING: ts-app has added the class-properties plugin to its build, but ember-cli-babel provides these by default now! You can remove the transforms, or the addon that provided them, such as @ember-decorators/babel-transforms.
- Building
Environment: test
- Building
- cleaning up
cleaning up...
- cleaning up
Built project successfully. Stored in "C:\Users\lifeart\Documents\repos\tmp\ts-app\tmp\class-tests_dist-RrzsVrR1.tmp".
ok 1 Chrome 74.0 - [2 ms] - ESLint | app: app/models/meta-model.js
ok 2 Chrome 74.0 - [0 ms] - ESLint | app: app.js
ok 3 Chrome 74.0 - [0 ms] - ESLint | app: resolver.js
ok 4 Chrome 74.0 - [0 ms] - ESLint | app: router.js
ok 5 Chrome 74.0 - [0 ms] - TemplateLint: ts-app/templates/application.hbs
ok 6 Chrome 74.0 - [0 ms] - ESLint | tests: test-helper.js
ok 7 Chrome 74.0 - [110 ms] - Unit | Model | base model: it exists
ok 8 Chrome 74.0 - [26 ms] - Unit | Model | meta model: it exists
ok 9 Chrome 74.0 - [1 ms] - ember-qunit: Ember.onerror validation: Ember.onerror is functioning properly

1..9
# tests 9
# pass  9
# skip  0
# fail  0

# ok
- Building
WARNING: ts-app has added the class-properties plugin to its build, but ember-cli-babel provides these by default now! You can remove the transforms, or the addon that provided them, such as @ember-decorators/babel-transforms.
- Building
WARNING: ts-app has added the class-properties plugin to its build, but ember-cli-babel provides these by default now! You can remove the transforms, or the addon that provided them, such as @ember-decorators/babel-transforms.
- Building
Environment: test
- Building
- cleaning up
cleaning up...
- cleaning up
Built project successfully. Stored in "C:\Users\lifeart\Documents\repos\tmp\ts-app\tmp\class-tests_dist-u3kw1tiO.tmp".
ok 1 Chrome 74.0 - [2 ms] - ESLint | app: app/models/meta-model.js
ok 2 Chrome 74.0 - [1 ms] - ESLint | app: app.js
ok 3 Chrome 74.0 - [0 ms] - ESLint | app: resolver.js
ok 4 Chrome 74.0 - [0 ms] - ESLint | app: router.js
ok 5 Chrome 74.0 - [0 ms] - TemplateLint: ts-app/templates/application.hbs
ok 6 Chrome 74.0 - [0 ms] - ESLint | tests: test-helper.js
ok 7 Chrome 74.0 - [103 ms] - Unit | Model | base model: it exists
not ok 8 Chrome 74.0 - [27 ms] - Unit | Model | meta model: it exists
    ---
        expected: >
            addon-meta-model/baseModel
        stack: >
                at Object.<anonymous> (http://localhost:7357/assets/tests.js:67:14)
                at runTest (http://localhost:7357/assets/test-support.js:5618:30)
                at Test.run (http://localhost:7357/assets/test-support.js:5604:6)
                at http://localhost:7357/assets/test-support.js:5831:12
                at processTaskQueue (http://localhost:7357/assets/test-support.js:5197:24)
                at advanceTaskQueue (http://localhost:7357/assets/test-support.js:5182:4)
        negative: >
            false
        browser log: |
    ...
ok 9 Chrome 74.0 - [0 ms] - ember-qunit: Ember.onerror validation: Ember.onerror is functioning properly

1..9
# tests 9
# pass  8
# skip  0
# fail  1
Testem finished with non-zero exit code. Tests failed.

@lifeart
Copy link
Author

lifeart commented May 16, 2019

// broken case ts input

// lib\side-models\addon\models\meta-model.ts
import DS from 'ember-data';

export default class MetaModel extends DS.Model.extend({
  name: 'addon-meta-model'
}) {
  // normal class body definition here
}


// DO NOT DELETE: this is how TypeScript knows how to look up your models.
declare module 'ember-data/types/registries/model' {
  export default interface ModelRegistry {
    'meta-model': MetaModel;
  }
}


// app\models\base-model.ts
import MetaModel from 'side-models/models/meta-model';

export default class BaseModel extends MetaModel.extend({
  baseName: 'baseModel'
}) {
  // normal class body definition here
}

// DO NOT DELETE: this is how TypeScript knows how to look up your models.
declare module 'ember-data/types/registries/model' {
  export default interface ModelRegistry {
    'base-model': BaseModel;
  }
}


// app\models\meta-model.ts

import BaseModel from './base-model';
import { computed } from "@ember/object";

export default class MetaModel extends BaseModel.extend({
  fullName: computed('name', 'baseName', function(){
    return this.name + '/' + this.baseName;
  })
}) {
  // normal class body definition here
}

// DO NOT DELETE: this is how TypeScript knows how to look up your models.
declare module 'ember-data/types/registries/model' {
  export default interface ModelRegistry {
    'metaModel': MetaModel;
  }
}

// broken case js output

;define("side-models/models/meta-model", ["exports", "ember-data"], function (_exports, _emberData) {
  "use strict";

  Object.defineProperty(_exports, "__esModule", {
    value: true
  });
  _exports.default = void 0;

  class MetaModel extends _emberData.default.Model.extend({
    name: 'addon-meta-model'
  }) {} // normal class body definition here
  // DO NOT DELETE: this is how TypeScript knows how to look up your models.


  _exports.default = MetaModel;
});
;define("ts-app/models/base-model", ["exports", "side-models/models/meta-model"], function (_exports, _metaModel) {
  "use strict";

  Object.defineProperty(_exports, "__esModule", {
    value: true
  });
  _exports.default = void 0;

  class BaseModel extends _metaModel.default.extend({
    baseName: 'baseModel'
  }) {} // normal class body definition here
  // DO NOT DELETE: this is how TypeScript knows how to look up your models.


  _exports.default = BaseModel;
});
;define("ts-app/models/meta-model", ["exports", "side-models/models/meta-model"], function (_exports, _metaModel) {
  "use strict";

  Object.defineProperty(_exports, "__esModule", {
    value: true
  });
  Object.defineProperty(_exports, "default", {
    enumerable: true,
    get: function () {
      return _metaModel.default;
    }
  });
});

// valud case js output

;define("side-models/models/meta-model", ["exports", "ember-data"], function (_exports, _emberData) {
  "use strict";

  Object.defineProperty(_exports, "__esModule", {
    value: true
  });
  _exports.default = void 0;

  class MetaModel extends _emberData.default.Model.extend({
    name: 'addon-meta-model'
  }) {} // normal class body definition here
  // DO NOT DELETE: this is how TypeScript knows how to look up your models.


  _exports.default = MetaModel;
});


;define("ts-app/models/base-model", ["exports", "side-models/models/meta-model"], function (_exports, _metaModel) {
  "use strict";

  Object.defineProperty(_exports, "__esModule", {
    value: true
  });
  _exports.default = void 0;

  class BaseModel extends _metaModel.default.extend({
    baseName: 'baseModel'
  }) {} // normal class body definition here
  // DO NOT DELETE: this is how TypeScript knows how to look up your models.


  _exports.default = BaseModel;
});
;define("ts-app/models/meta-model", ["exports", "ts-app/models/base-model"], function (_exports, _baseModel) {
  "use strict";

  Object.defineProperty(_exports, "__esModule", {
    value: true
  });
  _exports.default = void 0;

  class MetaModel extends _baseModel.default.extend({
    fullName: Ember.computed('name', 'baseName', function () {
      return this.name + '/' + this.baseName;
    })
  }) {} // normal class body definition here
  // DO NOT DELETE: this is how TypeScript knows how to look up your models.


  _exports.default = MetaModel;
});

@lifeart
Copy link
Author

lifeart commented May 16, 2019

so, for file ts-app/models/meta-model import of ts-app/models/base-model sometimes rewrited to side-models/models/meta-model and this is produce error.

@lifeart
Copy link
Author

lifeart commented May 16, 2019

may be related to emberjs/ember-cli-babel#240

@makepanic
Copy link

makepanic commented Jun 3, 2019

I think I'm seeing the same issue, but not in testing.

Somehow sometimes our prod build causes the consuming apps models to be overwritten by its addon models.

define("app/models/name", ["exports", "addon/models/name"], function(e, t) {
    Object.defineProperty(e, "__esModule", {
        value: !0
    }),
    Object.defineProperty(e, "default", {
        enumerable: !0,
        get: function() {
            return t.default
        }
    })
})

A correct prod build has more define dependencies than what's sometimes seen above.

Setup looks like this:

  • <app>/app/models/name.ts
  • <addon>/app/models/name.js
  • <addon>/addon/models/name.ts

edit: It seems like the error was caused by our CI deploying an old version. It might've been caused by the version using <addon>/app/models/name.ts back when migrating to e-c-ts@2

@lifeart
Copy link
Author

lifeart commented Jun 3, 2019

@makepanic yeah, now only one solution - if files have same name, both files MUST be .js

@richard-viney
Copy link

I've had a hard-to-reproduce problem that may be related to this.

Essentially, a production build of our Ember+TypeScript app would not have the application route in it, and this would break the app. The code in app/routes/application.ts just wasn't present anywhere in the built JS in dist/, and what appeared to be an empty/no-op application route was present instead.

The app in question is fully TS, i.e. there are no .js files in app/ at all.

I've encountered this several times over the last few months, but it was not deterministic and a quick rebuild would fix the problem so it wasn't investigated further. I've seen it in both development and production builds. If it happens again I'll save the output.

@makepanic
Copy link

makepanic commented Jul 31, 2019

We're still seeing this issue.
The bad thing is that it seems like it can work correctly during testing but generate a broken production build (there are only *.js files in the addons /app directory).

Is this a ember-cli-typescript or ember-cli issue?

I'm currently trying to avoid any conflicts between my app and any addons and will report back how it works out.

@lifeart
Copy link
Author

lifeart commented Jul 31, 2019

@makepanic all you need - have .js extension for overlapping names.
app/models/main.js + addon/models/main.js

@lifeart
Copy link
Author

lifeart commented Jul 31, 2019

@richard-viney ember-cli/ember-ajax#428
ember-simple-auth expose application.js route into app, and if you have application.ts in app, it may be issue. rename application.ts to application.js - this should solve your problem

@makepanic
Copy link

makepanic commented Jul 31, 2019

@makepanic all you need - have .js extension for overlapping names.
app/models/main.js + addon/models/main.js

How would one handle types in this context? I'd like to not lose the ability to type things.
Would one have to create dummy files that are imported by the js files?

e.g. when configuring the ember-ajax service one usually extends the ajax service. It means i have to either:

  • create ajax.js, create _ajax.ts which ajax.js reexports
  • create my-ajax.ts which extends ember-ajax/services/ajax and use service:my-ajax instead of service:ajax

@lifeart
Copy link
Author

lifeart commented Jul 31, 2019

@makepanic yah, my suggestion - only way to get consistent builds. But, yes, cons - you loose types.

@lolmaus
Copy link

lolmaus commented Jul 31, 2019

Related: ember-cli/ember-ajax#428 (comment)

@chriskrycho
Copy link
Member

The summary from that issue:

Having .js supplied to a specific path inside the app namespace by both an addon and the app itself, when one of them is using TypeScript to generate that file, creates a caching problem. When addons inject .js files into the app namespace, and we're compiling .ts files to .js files in the consuming app (always in the app namespace), or vice versa, we can end up in a situation where Ember CLI sees that there is already a .js file there, so it ignores the one generated from .ts. Which one it picks up seems to be semi-random (at least: we haven't chased it down to see if it's consistently replicable).

I'm closing this in favor of #780, where we'll track getting this into docs. Thank you for the report and helping us sort through it! Hopefully getting it into the docs will help users who encounter it.

@lifeart
Copy link
Author

lifeart commented Jul 31, 2019

@chriskrycho I think it should be redlined and pinned in the docs - because this is floating behaviour and can create serious damage for business.

@chriskrycho
Copy link
Member

@lifeart I'm definitely going to make it a loud and clear note, but there's only so much we can do—people have to find and read the docs. 🤷‍♂

@lifeart
Copy link
Author

lifeart commented Jul 31, 2019

@chriskrycho is it make sense to search possible static namespace overlaps on ember-cli boot?
get all addons, check for exports, check app locals for possible .ts overlaps

@jamescdavis
Copy link
Member

FYI: .js/.ts collision detection (warning) has been released in v3.1.3

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

6 participants