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

Extensions and themes splited into node packages #899

Merged
merged 21 commits into from
Mar 22, 2018
Merged

Conversation

Igloczek
Copy link
Contributor

@Igloczek Igloczek commented Mar 20, 2018

I dropped support of npm for core development, b/c only yarn support workspaces (apart of other advantages)

All hacks related to manually running npm in the child folders are removed, b/c are not necessary anymore.

I also introduced some very basic packages naming convention:
@vue-storefront is the official namespace
@vue-storefront/theme-* is for themes
@vue-storefront/extension-* is for extensions

Packages, for now, are not published.

The whole process of splitting the codebase is not done yet, the core is still waiting.

I'm creating this PR mostly to test everything on few machines, before merging anything.

@Igloczek Igloczek requested review from pkarw and filrak March 20, 2018 16:58
@Igloczek Igloczek force-pushed the feature/npm-modules branch from 52c531d to f0548fe Compare March 20, 2018 16:59
@Igloczek Igloczek force-pushed the feature/npm-modules branch from 643eab4 to 03d5baf Compare March 21, 2018 13:16
@Igloczek Igloczek mentioned this pull request Mar 21, 2018
Copy link
Collaborator

@pkarw pkarw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me it's good. Can be merged to develop. I've tested it Yesterday, everything works just fine

core/app.js Outdated
@@ -46,7 +45,15 @@ export function createApp () {
render: h => h(App)
})

registerExtensions(_.union(extensionEntryPoints, config.requiredExtensions) || [], app, router, store, config) // TODO: use config or ENV variables
// TODO: use config or ENV variables
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pkarw do you know if this TODO is still necessary here?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't remember what did I mean by this :) remove it :)

@filrak
Copy link
Collaborator

filrak commented Mar 21, 2018

I Will test tomorrow on Windows and Linux

@filrak
Copy link
Collaborator

filrak commented Mar 22, 2018

Works for me on Windows/ElementaryOS, merging ;)

@filrak filrak merged commit 3c260e2 into develop Mar 22, 2018
@Igloczek Igloczek deleted the feature/npm-modules branch March 22, 2018 18:52
@theprivateuser
Copy link

Tested on Windows 10 and works great on except v-viewer has stopped working. It was working but now seems to be hung up with SSR type problems after yarn. I verified the version didn't change so I'm not quite sure what's up with that.

@Igloczek
Copy link
Contributor Author

@jimcreate78 Could you create a new issue with some scenario to reproduce it?

@theprivateuser
Copy link

I will try and stage the issue

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

Successfully merging this pull request may close these issues.

4 participants