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

How best to include Prebid.js as part of a bigger project? #1853

Closed
cwbeck opened this issue Nov 20, 2017 · 13 comments · Fixed by #3435
Closed

How best to include Prebid.js as part of a bigger project? #1853

cwbeck opened this issue Nov 20, 2017 · 13 comments · Fixed by #3435
Assignees
Labels
pinned won't be closed by stalebot

Comments

@cwbeck
Copy link

cwbeck commented Nov 20, 2017

We are moving to a new framework and thought it a good idea to ask how best to include Prebid.js library. In an ideal world, I would like to add it as a dependency to package.json and :-

import prebid from "prebid.js";

This leads to a whole load of issues: -

    WARNING in ./node_modules/prebid.js/src/native.js
    136:27-39 "export 'triggerPixel' was not found in './utils'
     @ ./node_modules/prebid.js/src/native.js
     @ ./node_modules/prebid.js/src/prebid.js
     @ ./src/AdManager.ts
    
    WARNING in ./node_modules/prebid.js/src/video.js
    16:34-47 "export 'videoAdapters' was not found in './adaptermanager'
     @ ./node_modules/prebid.js/src/video.js
     @ ./node_modules/prebid.js/src/prebid.js
     @ ./src/AdManager.ts
    
    WARNING in ./node_modules/prebid.js/src/sizeMapping.js
    28:4-20 "export 'logMessage' (imported as 'utils') was not found in './utils'
     @ ./node_modules/prebid.js/src/sizeMapping.js
     @ ./node_modules/prebid.js/src/adaptermanager.js
     @ ./node_modules/prebid.js/src/prebid.js
     @ ./src/AdManager.ts

Has anyone found a good way to include this, or is it just easier to stick to a build and shim it in?

  "shim": {
    "vendor/prebid": {
      "exports": "pbjs"
    }
  },

Just feels a bit horrible!

@mkendall07
Copy link
Member

I'm glad someone finally asked! I don't actually know the best practice here. Would you mind either posting a minimal build system you are using or describing the the setup in more detail? Open to suggestions here but my guess is that we'd need to distribute a "packaged" version of the core + add on modules to be able to do a direct import.

@cwbeck
Copy link
Author

cwbeck commented Nov 20, 2017

@mkendall07 - thanks for the very quick reply. I'm still prototyping a few ideas, we have literally just started on this, so there is very little code. This is just to have a single build to go in the header of the site that will include Prebid.js.

I think you are correct, it will just require a built version "prebid.js": "^0.33.0", included in this project.

{
  "name": "ad-manager",
  "version": "0.1.0",
  "description": "Ad Manager",
  "scripts": {
    "build": "webpack",
    "server": "node server.js"
  },
  "dependencies": {
    "prebid.js": "^0.33.0",
    "@types/node": "^8.0.53",
    "@types/jquery": "^3.2.16",
    "@types/react": "^16.0.25",
    "@types/react-dom": "^16.0.3",
    "@types/react-redux": "^5.0.13",
    "jquery": "^3.2.1",
    "ts-loader": "^3.1.1",
    "typescript": "^2.6.1",
    "webpack": "^3.8.1",
    "webpack-dev-server": "^2.9.4",
    "react": "^16.1.1",
    "react-redux": "^5.0.6",
    "react-dom": "^16.1.1",
    "redux": "^3.7.2"
  },
  "repository": {
    "type": "git",
    "url": "https://github.com/Venatus/ad-manager"
  },
  "keywords": [
    "ad manager"
  ],
  "author": "Chris Beck",
  "contributors": [
    "Chris Beck <chrisb@venatusmedia.com> (http://venatusmedia.com)"
  ]
}
{
  "compilerOptions": {
    "outDir": "./dist/",
    "sourceMap": true,
    "noImplicitAny": true,
    "module": "commonjs",
    "target": "es5",
    "jsx": "react",
    "allowJs": true,
    "checkJs" : false,
    "lib": ["es2015", "dom"] //required when trying to use jQuery @types
  },
  "include": [
    "src/**/*"
  ],
  "exclude": [
    "node_modules",
    "build"
  ]
}
import prebid from "prebid.js";

module AdManager {
    export class Load {
        constructor() {

            console.log(prebid);

        }
    }
}
new AdManager.Load();

@bretg
Copy link
Collaborator

bretg commented Nov 20, 2017

@snapwich - you're working on packaging enhancements... would that help address this item?

@cwbeck
Copy link
Author

cwbeck commented Nov 20, 2017

For now, I'm going to leave this as a basic Prebid module: -

import "path/to/build/dist/prebid.js"

declare const pbjs: any;

export default pbjs;

It would be great to properly bundle this in however 👍

@bretg
Copy link
Collaborator

bretg commented Nov 20, 2017

Snapwich is out this week, but will take a look next week.

@ETNOL
Copy link
Contributor

ETNOL commented Nov 21, 2017

@cwbeck Prebid.js doesn't have a default export (I don't believe) so importing prebid as a 'named' export will transpile to something unusable. Based on your output I'm guessing your not actually doing that or you're using some sort of TS setting that I'm not familiar with. I'd expect

import prebid from ‘prebidjs’; 
console.log(prebid);

to transpile to something similar to

var prebid = require(‘prebid’); 
console.log(prebid.default);

I doubt you're doing this though since you're not getting an error saying something about prebid.default being undefined.So just to clarify, have you tried import * as prebid from 'prebid.js' instead? No idea if it would work but I'm curious what the results would be.

@cwbeck
Copy link
Author

cwbeck commented Nov 30, 2017

@entol, sorry, I forgot to reply to this! Trying to include it as you suggested just results in warnings complaining about dependencies from prebid. I can see why this is the case now, it would take a built version to be provided - this is the behaviour of other libraries that allow require("bla"). You are also correct, this is TypeScript, but not caused by the language itself. An framework using an AMD loader or similar (Require etc) would suffer a similar issue.

@mmilleruva
Copy link
Collaborator

mmilleruva commented Jan 4, 2018

@cwbeck I just spent some time looking into this and here is what I ended up doing:

  1. Add prebid to your package.json
  2. Add a module.json file at the root of your repo documenting what adapters you want to include.
  3. Create a vendor folder at the root of your repo.
  4. Add a grunt/gulp/npm task to go into the node_modules/prebid.js build and copy the file. Here is a shell script that we use:
#!/bin/bash
cd node_modules/prebid.js
yarn install
node_modules/.bin/gulp build --modules=../../modules.json 
cp build/dist/prebid.js ../../vendor/prebid.js

Finally you just need to call import vendor/prebid (assuming you are using webpack) and it will get pulled into your bundle and set up on the window.

@cwbeck
Copy link
Author

cwbeck commented Jan 5, 2018

@mmilleruva correct. This is exactly how I have it set up now, although I symlink to my fork prebid.js - This is however not necessarily the best way of doing it.

@patrick-mcdougle
Copy link

You could also load prebid in a different script tag and mark it as an external in webpack. As long as script order is maintained (not using async or using defer) this should work too.

@stale
Copy link

stale bot commented Mar 6, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@snapwich
Copy link
Collaborator

snapwich commented Dec 7, 2018

A lot has changed to the point where this might be more possible than it was in the past. Going to take some looks about the feasibility here.

@snapwich snapwich reopened this Dec 7, 2018
@snapwich snapwich removed the wontfix label Dec 7, 2018
@stale
Copy link

stale bot commented Dec 22, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Dec 22, 2018
@stale stale bot closed this as completed Dec 29, 2018
@snapwich snapwich reopened this Jan 2, 2019
@stale stale bot removed the stale label Jan 2, 2019
@snapwich snapwich added the pinned won't be closed by stalebot label Jan 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pinned won't be closed by stalebot
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants