Feature/core package

[lcnogueira]

Summary

Resolves #375

Relevant technical choices

QA Steps

Checklist

• My code is tested and passes existing unit tests.
• My code has an appropriate set of unit tests which all pass.
• My code passes the linting.
• My code has proper inline documentation.
• I have manually tested this PR.

[lcnogueira]

Mudança necessária para que os pacotes internos reconhecessem esse como o pacote base do projeto.

[cypress]

---

Test summary

24 • 0 • 5 • 0

---

Run details

Project	plataforma-sabia
Status	Passed
Commit	0a4b2ed ℹ️
Started	Sep 17, 2020 8:44 PM
Ended	Sep 17, 2020 8:49 PM
Duration	05:17 💡
OS	Linux Ubuntu Linux - 16.04
Browser	Electron 80

View run in Cypress Dashboard ➡️

---

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

[lcnogueira]

Necessário deixar todos os pacotes na mesma versão para que o comando bootstrap do lerna realmente consiga instalar e linkar as dependências dos pacotes internos.

[lcnogueira]

cc @nicholasio @marcusmota @mateus4k @luizeboli @davioliveiira
Pessoal, precisamos discutir e avaliar em conjunto essa mudança, pois vai afetar bastante a estrutura do projeto. Por isso, estou marcando vocês aqui pois tem alguns pontos importantes que precisamos levar em consideração.

Ponto 1)
Por padrão, o NextJs não compila arquivos que são "externos" (não estão dentro do projeto Next em si). Por isso inicialmente, estava recebendo um erro de que deveria usar um loader (postei no slack antes) para transpilar o código. Após pesquisar, vi que o padrão é usar o next-transpile-modules pra transpilar o pacote do monorepo:

const withTM = require('next-transpile-modules')(['@sabia/core']);

module.exports = withTM({
     //restante da config do webpack que já existia aqui...
});

Com isso, já consigo importar pacotes do pacote core (ex: user/my-account/index.js):

Ponto 2)
Apesar da configuração acima, quando tentava importar o useAuth do pacote core, tomava o seguinte erro:

hooks can only be called inside the body of a function component

Apesar da mensagem, o erro não tem a ver com o uso incoreto do custom hook, mas sim do fato de, em algum momento, o projeto ter mais de um pacote do react/react-dom. Pesquisando, encontrei muuuuitas issues relacionadas ao problema. Seguem as mais relevantes:

• Hooks can only be called inside the body of a function component - external module vercel/next.js#9506
• Invalid hook call in 9.0.6 vercel/next.js#9022 (muito relevante)
• Hooks + multiple instances of React facebook/react#13991
• Can’t extract a custom hook (in codesandbox) facebook/react#14454

Issue aberta no projeto do next-transpile-modules:
martpie/next-transpile-modules#50 (muito relevante)

Outros projetos que usam o Next e se basearam na issue 9022:

• Fix #55 by moving react & react-dom dependencies one level up kyle-mccarthy/nest-next#58 (igual à solução que tentei aqui)
• Address the nextjs issue with react hooks bloom-housing/bloom#87

Repositório de exemplo exibindo o bug:

• https://github.com/baldurh/next-9.0.6-bug-repro

Para consulta posterior (serve de exemplo caso decidamos por fazer uma config específica no webpack do projeto web):

• https://github.com/vercel/next.js/blob/canary/examples/using-preact/next.config.js

De um modo geral, há principalmente 2 soluções possíveis:

1. Fazer "hoisting" dos pacotes, levando os pacotes do react e react-dom para a raiz e removendo dos pacotes internos, fazendo com que esses utilizem o do diretório raiz.
2. Fazer uma configuração específica no webpack, fazendo com que o alias desses pacotes apontem para o pacote iterno (e não do package core). Nesse caso, não faríamos o hoisting dos pacotes para a raiz

Confesso que estou em dúvida de qual seria a melhor solução. Inicialmente, pensei na solução 1, mas, talvez, para não mexermos em toda a estrutura,talvez fosse melhor usar a solução 2, configurando somente no webpack do projeto web onde procurar pelos pacotes do react, react-dom.

Eu realizei a config 1 para fazer um teste. Com isso, consegui passar a importar o pacote useAuth, porém, o projeto fica apresentando erro do lint:

O que acham? Algum insight? @nicholasio

P.S.: Ainda aplicarei a mesma solução que for decidida aqui para o pacote do styled-components, pois também está aparecendo um warning no console de que o pacote está sendo utilizado em duplicidade e, inclusive, aponta para uma seção da documentação que mostra exatamente a mesma mensagem. Essa seção mostra tanto a solução fazendo hoisting (solução1) quanto a solução com o webpack (solução 2)

Ainda irei:

• Verificar se o hoisting dos pacotes do react para o package raiz do monorepo afetou aquele ambiente.
• Verificar o restante do projeto web, para sair substituindo os pacotes atuais pelos pacotes do @sabia/core (no momento, só fiz a importação de teste que descrevi acima).
• Atualizar a documentação para remover a parte de instalação (npm install) diretamente dentro dos packages, pois isso vai gerar erros (não vai reconhecer o @sabia/core).

[luizeboli]

@lcnogueira excelente investigação, parabéns 🎉

Não entendi muito bem a segunda solução. Seria configurar o webpack do pacote core pra pegar o react/dom do pacote web, ou o pacote web procurar no pacote core? Pois você comenta que a segunda solução seria fazendo com que o alias desses pacotes aointem para o pacote da raiz (e não do package interno). e logo abaixo configurando somente no webpack do projeto web onde procurar pelos pacotes do react, react-dom. Iria subir essas dependências pra raiz?

A solução 1 parece ser a mais correta, mas se for complexo/demorado em relação a outra, podemos configurar o webpack. Considerando que estamos atrasados pode ser a melhor escolha nesse momento

[lcnogueira]

Oi @luizeboli , obrigado pelo retorno. De fato, ficou um pouco confuso a explicação (editei lá). A solução 2 seria fazer com que o pacote web use a dependência interna do react mesmo, e não a do pacote core. Nesse caso, nós não subiríamos as dependências do react para a raiz.

[nicholasio]

@lcnogueira o pacote core precisa declarar o react e react-dom como peerDependencies, já tentou isso?

[luizeboli]

@lcnogueira o pacote core precisa declarar o react e react-dom como peerDependencies, já tentou isso?

Pensei em sugerir isso, mas no primeiro link é uma das primeiras coisas que o usuário relata, que mesmo com peer deps não rolou

[lcnogueira]

Putz...eu fiz tantos testes que esqueci de confirmar esse detalhe, @nicholasio . Tinha visto no link que @luizeboli comentou também esse detalhe de que não funcionaria. Mas testei agora só por desencargo e funcionou! 🤷‍♂️

Então, voltei o código: deixei cada pacote com suas dependências (do jeito que já estava) e só deixei os pacotes do react, react-dom e styled-components como peerDependencies no pacote core 👍 🤝

[nicholasio]

Putz...eu fiz tantos testes que esqueci de confirmar esse detalhe, @nicholasio . Tinha visto no link que @luizeboli comentou também esse detalhe de que não funcionaria. Mas testei agora só por desencargo e funcionou! 🤷‍♂️

Então, voltei o código: deixei cada pacote com suas dependências (do jeito que já estava) e só deixei os pacotes do react, react-dom e styled-components como peerDependencies no pacote core 👍 🤝

Haha, o bom é que agora nunca mais esquece. srsrs Muito provavelmente o que aconteceu é que o npm install instalou o react como dep de fato no core e o next.js estava pegando duas versões do react. Quando vc colocar como peerDep ela não é tratado como uma dependência instalado, e no caso quem está consumindo essa dep é que precisa instalar.

[lcnogueira]

Ah, entendi. Agora não esqueço mais 👊 . Depois vi que lá no projeto do "let-me-fix" realmente está usando peerDependencies também.

[lcnogueira]

Um detalhe que percebi é que o eslint não tá reconhecendo essas peerDependencies dentro do pacote core. Ele reconhece o react no pacote web, mas não no próprio pacote core:

O código em si tá funcional: rodei o web, sem problemas. Gerei o build também, sem problemas.

[luizeboli]

Um detalhe que percebi é que o eslint não tá reconhecendo essas peerDependencies dentro do pacote core. Ele reconhece o react no pacote web, mas não no próprio pacote core:

O código em si tá funcional: rodei o web, sem problemas. Gerei o build também, sem problemas.

O erro que tá dando é o import/no-unresolved? Não vai reconhecer já que pro eslint a dependência não ta instalada..

Pra resolver, vc pode editar o config, e adicionar de 2 formas:

1. 'import/no-unresolved': [2, { ignore: ['^test-utils', 'ˆreact'] }],
2. Pega o peerDependencies do package.json e faz um spread das keys no ignore

[lcnogueira]

Com a configuração do monorepo, os pacotes serão sempre instalados pela raiz, através do comando lerna bootstrap (automaticamente executado após o install).

[lcnogueira]

Mapeia alguns pacotes para que sejam reconhecidos pelo Jest. Necessário porque, em alguns casos, importamos pacotes de dentro do core para fazer um mock. Ex.:

jest.spyOn(useAuth, 'default').mockReturnValue({
			user: {
				email,
			},
		});

Nesses casos, ele não vai usar alguns pacotes que estão como peerDependencies ou não estão incluídos no core. Nesse caso, mapeamos para que o jest use do próprio pacote web (o <rootDir> aponta para o diretório web).