Doesn't work with Embroider? (build-time/dynamic magic?)

[NullVoxPopuli]

Getting this error:

Not sure why though. It looks resolveable (in addon directory).

cc @ef4

[NullVoxPopuli]

Looks like this is the call site (concat'd vendor.js):

[NullVoxPopuli]

ah, with chrome, I got some more info:

[NullVoxPopuli]

This might be a "fix": though, I'd really like to know why this is needed in the first place -- maybe there is build-time magic doing something with the /-internal modules?

  return compatBuild(app, Webpack, {
      extraPublicTrees: additionalTrees,
      staticAddonTestSupportTrees: true,
      staticAddonTrees: true,
      staticHelpers: true,
      staticComponents: true,
      // splitAtRoutes: true,
      // skipBabel: [],
      compatAdapters: new Map([
        ['ember-destroyable-polyfill', EmberDestroyableCompatAdapter],
      ]),
    });

// .......

const { V1Addon } = require('@embroider/compat');
const { forceIncludeModule } = require('@embroider/compat/src/compat-utils');

class EmberDestroyableCompatAdapter extends V1Addon {
  get packageMeta() {
    let meta = super.packageMeta;
    meta = forceIncludeModule(meta, './-internal/patch-core-object');
    meta = forceIncludeModule(meta, './-internal/patch-meta');

    return meta;
  }
}

[rwjblue]

There are a few issues here:

1) we can avoid needing to require by moving the patch content into vendor/ and setting up transpilation for treeForVendor 2) we need to figure out what the "right" way to masquerade in Embroider land is since we basically want to provide @ember/destroyable module, but that doesn't really match our expected name. I think we can implement moduleName(){ return '@ember/destroyable'; }?

[ef4]

Yes, moduleName is supported, and probably the least bad way to masquerade. I can show a solution for this addon, but I have a question:

Is the require.has('@ember/destroyable') check redundant with the emberVersion.lt('4.0.0') check? Or are they asking subtly different questions? I would guess they are both just trying to ask the same question, which is "should the polyfill be inert?", but then why have both checks?

[ef4]

Relatedly: what is the plan for shipping native @ember/destroyable? Will it be a package like @glimmer/component or will it another api shim?

I'm guessing shim, because otherwise we wouldn't need a separate polyfill package, we could just make the real package work in older ember versions?

[rwjblue]

Is the require.has('@ember/destroyable') check redundant with the emberVersion.lt('4.0.0') check?

Yes, I think they are redundant.

What is the plan for shipping native @ember/destroyable? Will it be a package like @glimmer/component or will it another api shim?

It still somewhat TBD (implementation is not done yet), but I would suspect it would be a shim package (like all of the rest of @ember/*).

[rwjblue]

OK, the implementation now matches what other polyfills do. It is a straight up app.import from a vendor file.

[rwjblue]

@NullVoxPopuli - Mind testing the latest release (v2.0.1)? (Note: you must be using ember-cli-babel@7.22.1 or higher)