embroider-build / embroider

Compiling Ember apps into spec-compliant, modern Javascript.
MIT License
330 stars 137 forks source link

can't locate @ember-data/record-data/-private referred to in module rules #1322

Closed mansona closed 1 year ago

mansona commented 1 year ago

I'm running tests in ember-try embroider-safe and we're getting the following error:

Build Error (WaitForTrees)

can't locate @ember-data/record-data/-private referred to in module rules:{
  "dependsOnModules": [
    "@ember-data/record-data/-private"
  ]
}

I've done some debugging and I've found the error that is being thrown in the resolver.js

'Cannot find module '@ember-data/record-data/-private' from '/private/var/folders/r5/bw5frvnn5k794hk62w3vp7km0000gn/T/embroider/2daa8e/node_modules/.pnpm/@ember-data+store@4.8.4_k6kp2ds5o4a3bn46aja3did5ee/node_modules/@ember-data/store''

I had a look at the file on disk and it looks like it's right, there was no sibling to @ember-data/store for record-data and normal node resolution would be expected to fail.

mansona commented 1 year ago

I'm wondering if this is a complication created by the fact that that ember-data is now essentially a meta-package that depends on things like @ember-data/store and @ember-data/record-data, but @ember-data/store only has a peer-dependency on @ember-data/record-data 🤔

mkszepp commented 1 year ago

Beginning with ember-data v4.10.x (atm alpha version 15) there is necessary to remove the addon dependency rules, because they have moved files / modified the build process in ember-data https://github.com/embroider-build/embroider/blob/aa97453277272f533c97db174f6d4b1851e77f9d/packages/compat/src/addon-dependency-rules/ember-data.ts#L3-L18

As workaround for this problem, you can use this lines:

ember-cli-build.js

   packageRules: [
      {
        package: '@ember-data/store',
        addonModules: {
          '-private.js': {
            dependsOnModules: [],
          },
          '-private/system/core-store.js': {
            dependsOnModules: [],
          },
          '-private/system/model/internal-model.js': {
            dependsOnModules: [],
          },
        },
      },
    ],

After this fix, you get the next error, which i think is a problem in ember-data (starting with v.4.8.x). Error while processing route: index Assertion Failed: Expected store.createRecordDataFor to be implemented but it wasn't

With Ember data 4.8.x you need a other workaround to fix the build process https://github.com/emberjs/data/issues/8296#issuecomment-1314860277 But also with this workaround you are running in the error ´store.createRecordDataFor`

With ember-data v4.9.1 there is no workaround necessary for embroider, but there is necessary to solve this issue https://github.com/emberjs/data/issues/8396

Since now, i havn't found a complete workaround to fix ember-data with embroider (without making forks)

runspired commented 1 year ago

this should be closable. For 4.8+ embroider should fully deactivate all compat adapters for ember-data.

EmberData's own embroider build at this point specifies the following:

return require('@embroider/compat').compatBuild(app, Webpack, {
    skipBabel: [
      {
        package: 'qunit',
      },
    ],
    compatAdapters: new Map([
      ['@ember-data/store', null],
      ['@ember-data/record-data', null],
      ['@ember-data/serializer', null],
      ['@ember-data/adapter', null],
      ['@ember-data/model', null],
      ['@ember-data/debug', null],
      ['@ember-data/tracking', null],
      ['@ember-data/request', null],
      ['@ember-data/private-build-infra', null],
      ['@ember-data/canary-features', null],
      ['ember-data', null],
    ]),
    packageRules: [
      {
        package: '@ember-data/store',
        addonModules: {
          '-private.js': {
            dependsOnModules: ['@ember-data/json-api/-private'],
          },
        },
      },
    ],
  });
mkszepp commented 1 year ago

@runspired i think we can also remove all packageRules, when the app is using ember-data v4.11.x (tested in my app)

So, embroider doen't need anymore any workarounds for ember-data.

packageRules: [
      {
        package: '@ember-data/store',
        addonModules: {
          '-private.js': {
            dependsOnModules: [],
          },
          '-private/system/core-store.js': {
            dependsOnModules: [],
          },
          '-private/system/model/internal-model.js': {
            dependsOnModules: [],
          },
        },
      },
    ],
ef4 commented 1 year ago

We support version ranges on the package rules, so a PR updating the built-in package rules so they stop applying to new-enough versions would be great.

mkszepp commented 1 year ago

@ef4 PR created.. is there necessary to add also something for the compatAdapters?

esbanarango commented 1 year ago

What's the @embroider/* version expected to avoid this error? I'm getting this same error after bumping ember-data to 4.12.0?

    "@embroider/addon-shim": "^1.8.4",
    "@embroider/compat": "2.1.1",
    "@embroider/core": "2.1.1",
    "@embroider/macros": "^1.10.0",
    "@embroider/router": "2.0.0",
    "@embroider/util": "^1.10.0",
    "@embroider/webpack": "2.1.1",
NullVoxPopuli commented 1 year ago

@esbanarango at present, you need to use the unstable-tagged releases to get the work in this PR.

A proper release hasn't happened yet, but is planned

esbanarango commented 1 year ago

Unfortunately, latest unstabled-tagged release completely breaks the app:

    "@embroider/addon-shim": "1.8.5-unstable.3a9d8ad",
    "@embroider/compat": "2.1.2-unstable.3a9d8ad",
    "@embroider/core": "2.1.2-unstable.3a9d8ad",
    "@embroider/macros": "1.10.1-unstable.3a9d8ad",
    "@embroider/router": "2.0.1-unstable.3a9d8ad",
    "@embroider/util": "1.10.1-unstable.3a9d8ad",
    "@embroider/webpack": "2.1.2-unstable.3a9d8ad",
SignalUi_Tests
NullVoxPopuli commented 1 year ago

I mean, it's unstable -- but also, it can't find app/app.ts, which could hint at a goofiness with your project.

vlascik commented 1 year ago

as a workaround, you can delete the node_modules/@embroider/compat/src/addon-dependency-rules/ember-data.js file.

mkszepp commented 1 year ago

the workaround, without removing files is, to add packageRules like described here https://github.com/embroider-build/embroider/issues/1322#issuecomment-1445804265 So you don't need to use the "unstable" version

packageRules: [
      {
        package: '@ember-data/store',
        addonModules: {
          '-private.js': {
            dependsOnModules: [],
          },
          '-private/system/core-store.js': {
            dependsOnModules: [],
          },
          '-private/system/model/internal-model.js': {
            dependsOnModules: [],
          },
        },
      },
    ],
mkszepp commented 1 year ago

but we run in an other problem with ember-data :(, ember-data is atm not anymore compatible with embroider :(

grafik

Error while processing route: index this.requestManager is undefined request@webpack://app-name/./node_modules/@ember-data/store/index-8b77b852.js?:5453:20
query@webpack://app-name/./node_modules/@ember-data/store/index-8b77b852.js?:6398:26
query@webpack://app-name/./services/store.js?:69:33
authenticate@webpack://app-name/./services/session-user.js?:43:23
beforeModel@webpack://app-name/./routes/rg.js?:39:28
runBeforeModelHook@http://localhost:5600/nt/assets/vendor.js:45450:31
resolve/<@http://localhost:5600/nt/assets/vendor.js:45374:26
invokeCallback@http://localhost:5600/nt/assets/vendor.js:47160:17
publish@http://localhost:5600/nt/assets/vendor.js:47146:23
@http://localhost:5600/nt/assets/vendor.js:42721:62
invoke@http://localhost:5600/nt/assets/vendor.js:41306:16
flush@http://localhost:5600/nt/assets/vendor.js:41221:19
flush@http://localhost:5600/nt/assets/vendor.js:41384:21
_end@http://localhost:5600/nt/assets/vendor.js:41825:34
Backburner/this._boundAutorunEnd@http://localhost:5600/nt/assets/vendor.js:41549:14

Edit: Created a Issue in ember-data https://github.com/emberjs/data/issues/8565

runspired commented 1 year ago

^ note that this last comment is not an issue with embroider but an issue with the app extending the store from @ember-data/store instead of ember-data/store when using ember-data which results in incomplete configuration.