odoo / o-spreadsheet

Other
195 stars 44 forks source link

npm version doesn't work: "This is a abstract store for Notifications, it cannot be instantiated." #4163

Open valpackett opened 6 months ago

valpackett commented 6 months ago

Version (please indicate which version you are using): 17.2.5, tried some others as well

Platform (OS and Browser + version): Linux, Firefox 125

Describe the bug

The code published on npm crashes in the following way upon instantiation:

Error: This is a abstract store for Notifications, it cannot be instantiated.
Did you forget to inject your store instance?

const stores = useStoreProvider();
stores.inject(MyMetaStore, storeInstance);
    MetaStore o-spreadsheet.esm.js:8084
    build o-spreadsheet.esm.js:8064
    instantiate o-spreadsheet.esm.js:8041
    get o-spreadsheet.esm.js:8036
    useStore o-spreadsheet.esm.js:8132
    setup o-spreadsheet.esm.js:56731
    ComponentNode owl.es.js:2354
    createComponent owl.es.js:5758
    template chunk-JBWWRFNB.js:23
    _render owl.es.js:1719
    render owl.es.js:1711
    initiateRender owl.es.js:2376
    mountComponent owl.es.js:2360
    mountNode owl.es.js:5688
    mount owl.es.js:5659
    <anonymous> index.html:269
    whenReady owl.es.js:306
    <anonymous> index.html:259

Spreadsheet.setup is defined like this in 17.2.5 npm release:

    setup() {
        const stores = useStoreProvider();
        stores.inject(ModelStore, this.model);
        this.notificationStore = useStore(NotificationStore);
        this.composerFocusStore = useStore(ComposerFocusStore);
        this.sidePanel = useStore(SidePanelStore);
        this.keyDownMapping = {
            "CTRL+H": () => this.sidePanel.toggle("FindAndReplace", {}),
            "CTRL+F": () => this.sidePanel.toggle("FindAndReplace", {}),
        };
        const fileStore = this.model.config.external.fileStore;

The 17.2.5 tag on GitHub, which actually refers to the 17.0.20 commit right now, does not contain any NotificationStore lines:

https://github.com/odoo/o-spreadsheet/blob/065ac988cd2fdf18bf2cd8b1d68660c3a45c387f/src/components/spreadsheet/spreadsheet.ts#L286-L297

and does work fine.

To Reproduce Steps to reproduce the behavior:

  1. npm i --save @odoo/o-spreadsheet
  2. Try to build a basic example using the documentation
rrahir commented 6 months ago

Thanks for the report :) The release script seems to be at fault here, I will investigate it right away.

rrahir commented 4 months ago

Hi! Just to let you know that we updated the documentation properly with the minimal setup required to start a spreadsheet. Specifically, there was a missing step concerning the setup of the NotificationStore. In the meantime, we also changed that behaviour so you don't have to setup any store before instanciating a Spreadsheet component.

Could you let us know if you can manage to build a basic example with these changes?

Regards,

valpackett commented 4 months ago

Thanks!! Seems to no longer be an issue in npm release 17.3.4.

Another issue though is that the package.json "module" field does not match the actual ESM build file name (.es.js vs .esm.js), breaking tools like Vite that parse npm packages. (One letter to fix…)

image

rrahir commented 3 months ago

Indeed, I will fix that right away :)