derekpitt / fw-model

1 stars 4 forks source link

Getting TypeError: Reflect.getMetadata is not a function #9

Open lemoustachiste opened 5 years ago

lemoustachiste commented 5 years ago

I'm trying to build with Rollup, as I run the build code, it fails on this:

var classType = Reflect.getMetadata("design:type", target, key);

it looks like getMetadata is not part of the native definition of Reflect: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Reflect and could be found here: https://www.npmjs.com/package/reflect-metadata

But it does not seem to be a dependency of this project, however, our build with Webpack does not fail?

cc @raiseandfall

lemoustachiste commented 5 years ago

I have found why webpack works, we take reflect-metadata as an entry

lemoustachiste commented 5 years ago

could we make fw-model embed the reflect-metadata bundle?

derekpitt commented 5 years ago

I think we can make that happen.

Yeah, we don't even have it listed as a peer..

Before I make a decision how to do this, let me look at our other builds.. we have a few packages that use reflect-metadata, and I want to take a peek at your setup too :)

What are you trying to do with rollup?

lemoustachiste commented 5 years ago

Thanks Derek,

our webpack config is specifying reflect-metadata as entry point:

let entry = [
  'regenerator-runtime/runtime',
  'reflect-metadata',
  'babel-polyfill',
  'event-source-polyfill',
  `./scss/main-${PRODUCT}.scss`,
  `./bootstrap-${PRODUCT}.ts`
];
module.exports = {
  context: path.join(__dirname, 'src'),
  entry: entry,
  output: {
    path: path.join(__dirname, clientPath),
    publicPath: publicPath,
    filename: PROD ? 'bundle-[hash].js' : 'bundle.js',
    chunkFilename: PROD ? '[id].bundle.[chunkhash].js' : '[id].bundle.js',
    sourceMapFilename: '[file].map'
}
...

We are trying to improve our build time and overhead with Rollup, because moving to webpack 4 is quite painful

lemoustachiste commented 5 years ago

So I made a fork and embedded the code, I had to change the build process because Gulp was only adding require('reflect-metadata') which would later fail if not handled specifically. So to make my life easier I also used rollup to build the output. I could make a PR if you think it's ok.

However I have been facing another issue which is not so easily solved. And after spending quite some time on it, I am curious if you might have some guidance to understand what's happening.

I see these errors in the console:

fromClass passed in type is undefined. is it defined above the calling class?

This is a stack trace:

  fromClass http://localhost:3050/bundle.js:14658 
  DecorateProperty http://localhost:3050/bundle.js:13825 
  decorate http://localhost:3050/bundle.js:13395 
  __decorate http://localhost:3050/bundle.js:21 
  <anonymous> http://localhost:3050/bundle.js:14737 

This is one of the calling code in the bundle:

__decorate([
    fromClass,
    __metadata("design:type", Saml2Settings)
], IdentityProvider.prototype, "saml2Settings", void 0);

right above it are the class definitions:

class Saml2Settings {
}
class IdentityProvider extends OrganizationBase {
}

So I am quite confused by the error message (which I saw is custom). I tried to follow through what's happening, but I didn't really see anything wrong. I added 2 console logs in fromClass in the bundle:

function fromClass(target, key, descriptor) {
  console.log(JSON.stringify(target));
  console.log(target, key);

which output the following

screenshot 2019-01-09 at 15 03 59

What it looks like to me is that we are trying to read an object that's not ready yet, but that's all I can make of it.

With a webpack build, the code looks similar (although ES5). And this is not related with this issue, and I don't necessarily think it is a general issue with fw-model, but maybe you have more clues/understanding as to what should happen at runtime and that could help figure out what's wrong with the rollup build.

Thanks

derekpitt commented 5 years ago

Hey, sorry I wasn't able to get back to you yesterday.

Yeah, your fork is looking good! The rollup build is something i've had on my todo list for a little bit, but haven't been able to sit down and do it. So thank you!!! Go ahead and make a PR, and we can work on the other things that need to happen with the build (like outputting the typescript def).

As for that error, yes, it is exactly what you describe. It will usually occur when you have code like this:

class Class1 {
  @fromClass prop: Class2;
}

// later on...

class Class2 {}

class1 will have it's decorators processed first, before class2 is seen. Moving class2 above class1 will fix it.

So I would have to see more of your code to see why that is happening, since it looks like it is setup correctly.

This is awesome! Thank you!!

lemoustachiste commented 5 years ago

yeah that's how it's defined in the src code:

export class Saml2Settings {
  metadataUrl?: string;
  isFederation?: boolean;
  uniqueIdAttribute?: string;
  emailAttribute?: string;
  givenNameAttribute?: string;
  surnameAttribute?: string;
}

export interface IIdentityProvider extends IOrganizationBase {
  name: string;
  entityId: string;
  saml2Settings: Saml2Settings;
}

export class IdentityProvider extends OrganizationBase {
  name: string;
  entityId: string;
  @fromClass saml2Settings: Saml2Settings;
}

To be noted that this is the example I gave, but all fromClass decorations seem to fail with rollup in this case.

I will try with a simpler case to see

lemoustachiste commented 5 years ago

So I made a basic repro: https://github.com/lemoustachiste/fw-model-rollup

I indeed seem to be able to get the same issue:

screenshot 2019-01-09 at 16 52 44
derekpitt commented 5 years ago

alright cool! i'll check it out and see whats up

derekpitt commented 5 years ago

I see what's going on!

We do rely on some compiler options with typescript:

so if you change your rollup config to be:

typescript({ emitDecoratorMetadata: true, experimentalDecorators: true }),

that will go away

lemoustachiste commented 5 years ago

Thanks Derek,

That's great info, and indeed I fixed the issue in the repro. However it still remains in my actual build. So now I'm exploring other things, like conflicts with the rest of the build (we are using Aurelia, so there might be weird things happening with it).

Anyway, it put me back on the trail so thanks for that. I'll keep you posted if I (hopefully) get somewhere