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

[Bug] Ember data doens't work with embroider starting with 4.8.x #8396

Closed
mkszepp opened this issue Jan 18, 2023 · 11 comments
Closed

[Bug] Ember data doens't work with embroider starting with 4.8.x #8396

mkszepp opened this issue Jan 18, 2023 · 11 comments

Comments

@mkszepp
Copy link
Contributor

mkszepp commented Jan 18, 2023

Description

There is not anymore possible to use ember-data with embroider, starting with 4.8.x.

In v4.9.1 we get this error:
Error Error while processing route: index Assertion Failed: Expected store.createRecordDataFor to be implemented but it wasn't

createRecordDataFor(
identifier: StableRecordIdentifier,
storeWrapper: RecordDataStoreWrapper
): RecordData | RecordDataV1 {
if (HAS_RECORD_DATA_PACKAGE) {
// we can't greedily use require as this causes
// a cycle we can't easily fix (or clearly pin point) at present.
//
// it can be reproduced in partner tests by running
// node ./scripts/packages-for-commit.js && pnpm test-external:ember-observer
if (_RecordData === undefined) {
_RecordData = (
importSync('@ember-data/record-data/-private') as typeof import('@ember-data/record-data/-private')
).RecordData;
}

This is because the private-build-infra HAS_RECORD_DATA_PACKAGE says, that the package @ember-data/record-data is not present.

HAS_RECORD_DATA_PACKAGE: '@ember-data/record-data',

Fix

We can fix this problem by replacing the line with HAS_RECORD_DATA_PACKAGE: '@ember-data/record-data/-private',

@mkszepp
Copy link
Contributor Author

mkszepp commented Jan 18, 2023

In ember-data v4.10.x there is necessary to replace all moduleExists("@ember-data/json-api") with moduleExists("@ember-data/json-api/-private"), so it works also with embroider

@mkszepp
Copy link
Contributor Author

mkszepp commented Jan 20, 2023

I have checked again this problem and the fix for this issue is better to add index.ts in package/record-data and replace all imports of @ember-data/record-data/-private with '@ember-data/record-data' in v4.9.1. After that the record-data packages works also with embroider.

I think we can do the same thing for @ember-data/json-api in v.4.10.x.

@ztjohns
Copy link

ztjohns commented Jan 20, 2023

I have checked again this problem and the fix for this issue is better to add index.ts in package/record-data and replace all imports of @ember-data/record-data/-private with '@ember-data/record-data' in v4.9.1. After that the record-data packages works also with embroider.

I think we can do the same thing for @ember-data/json-api in v.4.10.x.

Is this something we would need to do on a developers side while trying to update to the latest stable tag or is this more internal to embroider and ember-data.

I recently asked about this issue and discord and was told to roll back to 4.7.3 for ED for the time being.

Thank you!

@mkszepp
Copy link
Contributor Author

mkszepp commented Jan 21, 2023

@ztjohns the bug for v4.9 must be fixed in ember-data (pr is already open)... after that, it should be possible to use with embroider without workaround in your project.

For v4.10 we need changes in embroider + the fix in ember data. See embroider-build/embroider#1322 (comment)

I'm atm on 4.4, because 4.7 has varios bugs, which will not be backportet from 4.8/4.9 (for example identifier bug + problem with more than 1500 records). Beginning with 4.5+ my app was always buggy, so i recommend you to stay on 4.4.

When this bug is fixed in latest relesed ember-data version, i think that we should also backport all fixes to the lts 4.8.

A workaround in your project for 4.9 could be to create the index.ts or replace the package check string while build process. I think more is not necessary.

A workaround, maybe not perfect, but it should be work for 4.9.1:
ember-cli-build.js (add before module.exports)

const fs = require('fs');
fs.writeFileSync(
  `node_modules/@ember-data/private-build-infra/addon/available-packages.ts`,
  `export default {
  HAS_EMBER_DATA_PACKAGE: 'ember-data',
  HAS_STORE_PACKAGE: '@ember-data/store',
  HAS_MODEL_PACKAGE: '@ember-data/model',
  HAS_RECORD_DATA_PACKAGE: '@ember-data/record-data/-private',
  HAS_ADAPTER_PACKAGE: '@ember-data/adapter',
  HAS_SERIALIZER_PACKAGE: '@ember-data/serializer',
  HAS_DEBUG_PACKAGE: '@ember-data/debug',
};`
);

@Turbo87
Copy link
Member

Turbo87 commented Jan 26, 2023

related: #8296 (comment) :)

@runspired
Copy link
Contributor

the issue here is likely in embroider. These addons function as normal traditional ember addons. Its unclear why that would break in more recent embroider versions, especially when the embroider tests in this repo are passing...

@mkszepp
Copy link
Contributor Author

mkszepp commented Feb 12, 2023

@runspired atm ember data has only one test for embroider if the build works, but it is no test for accessing to a model/store... right?
Maybe there is necessary to add some tests for that, so also ember data will be failed under embroider...

Normally other packages are using the same test like without embroider, but ember data doens't do this atm...

@rreckonerr
Copy link

The same issue is blocking me from upgrading from 4.4 to 4.8

@runspired
Copy link
Contributor

@mkszepp are you on discord? would love to sync up and see if we can get to the root of some of this

@runspired
Copy link
Contributor

pretty sure @embroider/test-setup is not compatible currently with EmberApp (only EmberAddon) and therefore isn't usable by us. See glimmerjs/glimmer.js#406 for an error encountered trying to setup that way.

@mkszepp
Copy link
Contributor Author

mkszepp commented Feb 27, 2023

@runspired thank, confirm that it works without any workaround, package rules... in v4.11.x.

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